Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Early close resource file descriptors #95

Merged
merged 6 commits into from
Aug 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ jobs:
strategy:
fail-fast: false
matrix:
os: [ubuntu-20.04, ubuntu-22.04]
compiler: [[gcc, g++], [clang, clang++]]
os: [ubuntu-20.04, ubuntu-22.04, ubuntu-24.04]
compiler: [{cc: gcc, cxx: g++}, {cc: clang, cxx: clang++}]
timeout-minutes: 30

steps:
Expand All @@ -26,8 +26,8 @@ jobs:
./yrmcdsd &
sleep 1
env:
CC: ${{ matrix.compiler[0] }}
CXX: ${{ matrix.compiler[1] }}
CC: ${{ matrix.compiler.cc }}
CXX: ${{ matrix.compiler.cxx }}
- name: Run tests
run: |
make YRMCDS_SERVER=localhost tests
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
# Editors
*~
.*.swp
.vscode/settings.json

# Directories
html
Expand Down
18 changes: 18 additions & 0 deletions .vscode/c_cpp_properties.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"configurations": [
{
"name": "Linux",
"includePath": [
"${workspaceFolder}/**"
],
"defines": [
"CACHELINE_SIZE=32"
],
"compilerPath": "/usr/bin/gcc",
"cStandard": "c17",
"cppStandard": "gnu++17",
"intelliSenseMode": "linux-gcc-x64"
}
],
"version": 4
}
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# Makefile for yrmcds
# Prerequisites: gcc 4.8+ or clang 3.3+
# Prerequisites: gcc 11.1+ or clang 14+

PREFIX = /usr/local
DEFAULT_CONFIG = $(PREFIX)/etc/yrmcds.conf
Expand All @@ -17,7 +17,7 @@ OPTFLAGS = -O2 #-flto
DEBUGFLAGS = -gdwarf-3 #-fsanitize=address
WARNFLAGS = -Wall -Wnon-virtual-dtor -Woverloaded-virtual
CPUFLAGS = #-march=core2 -mtune=corei7
CXXFLAGS = -std=gnu++11 $(OPTFLAGS) $(DEBUGFLAGS) $(shell getconf LFS_CFLAGS) $(WARNFLAGS) $(CPUFLAGS)
CXXFLAGS = -std=gnu++17 $(OPTFLAGS) $(DEBUGFLAGS) $(shell getconf LFS_CFLAGS) $(WARNFLAGS) $(CPUFLAGS)
LDFLAGS = -L. $(shell getconf LFS_LDFLAGS)
LDLIBS = $(shell getconf LFS_LIBS) -lyrmcds $(LIBTCMALLOC) -latomic -lpthread

Expand Down
30 changes: 23 additions & 7 deletions cybozu/reactor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@ const int POLLING_TIMEOUT = 100; // milli seconds

namespace cybozu {

void resource::invalidate_and_close_() {
bool expected = true;
if( ! m_valid.compare_exchange_strong(expected, false) )
return;

// no need to read-lock `m_lock` here because `m_fd` will not be
// closed before this function calls `reactor::request_removal`.
on_invalidate(m_fd);
m_reactor->request_removal(*this);
}

reactor::reactor():
m_fd( epoll_create1(EPOLL_CLOEXEC) ), m_running(true)
{
Expand All @@ -37,7 +48,7 @@ void reactor::add_resource(std::unique_ptr<resource> res, int events) {
if( res->m_reactor != nullptr )
throw std::logic_error("<reactor::add_resource> already added!");
res->m_reactor = this;
const int fd = res->fileno();
const int fd = res->m_fd;
struct epoll_event ev;
ev.events = events | EPOLLET;
ev.data.fd = fd;
Expand All @@ -47,7 +58,7 @@ void reactor::add_resource(std::unique_ptr<resource> res, int events) {
}

void reactor::modify_events(const resource& res, int events) {
const int fd = res.fileno();
const int fd = res.m_fd;
struct epoll_event ev;
ev.events = events | EPOLLET;
ev.data.fd = fd;
Expand Down Expand Up @@ -78,12 +89,17 @@ void reactor::remove_resource(int fd) {
dump_stack();
throw std::logic_error("bug in remove_resource");
}
auto res = it->second.get();
m_garbage.emplace_back( std::move(it->second) );
m_resources.erase(it);
if( epoll_ctl(m_fd, EPOLL_CTL_DEL, fd, NULL) == -1 )
throw_unix_error(errno, "epoll_ctl(EPOLL_CTL_DEL)");
m_readables.erase(std::remove(m_readables.begin(), m_readables.end(), fd),
m_readables.end());

if( ! res->try_close() ) {
logger::debug() << "failed to close fd (will be closed later): " << fd;
}
}

void reactor::poll() {
Expand All @@ -93,7 +109,7 @@ void reactor::poll() {
std::back_inserter(m_readables_copy));
m_readables.clear();
for( int fd: m_readables_copy ) {
if( ! m_resources[fd]->on_readable() )
if( ! m_resources[fd]->on_readable(fd) )
remove_resource(fd);
}
m_readables_copy.clear();
Expand All @@ -120,24 +136,24 @@ void reactor::poll() {
const int fd = ev.data.fd;
resource& r = *(m_resources[fd]);
if( ev.events & EPOLLERR ) {
if( ! r.on_error() )
if( ! r.on_error(fd) )
remove_resource(fd);
continue;
}
if( ev.events & EPOLLHUP ) {
if( ! r.on_hangup() ) {
if( ! r.on_hangup(fd) ) {
remove_resource(fd);
continue;
}
}
if( ev.events & EPOLLIN ) {
if( ! r.on_readable() ) {
if( ! r.on_readable(fd) ) {
remove_resource(fd);
continue;
}
}
if( ev.events & EPOLLOUT ) {
if( ! r.on_writable() )
if( ! r.on_writable(fd) )
remove_resource(fd);
}
}
Expand Down
Loading
Loading