weaselab/weaseldb
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix EINTR handling

Browse Source
This commit is contained in:
Andrew Noyes
2025-08-23 17:28:34 -04:00
parent 5b4a28a8ca
commit a820efa2e6
6 changed files with 131 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/connection.cpp
View File

@@ -28,7 +28,10 @@ Connection::~Connection() {
if (auto server_ptr = server_.lock()) {
server_ptr->active_connections_.fetch_sub(1, std::memory_order_relaxed);
}
int e = close(fd_);
int e;
do {
e = close(fd_);
} while (e == -1 && errno == EINTR);
if (e == -1) {
perror("close");
std::abort();

35
src/main.cpp
View File

@@ -48,7 +48,10 @@ std::vector<int> create_listen_sockets(const weaseldb::Config &config) {
addr.sun_family = AF_UNIX;
if (config.server.unix_socket_path.length() >= sizeof(addr.sun_path)) {
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
std::fprintf(stderr, "Unix socket path too long\n");
std::abort();
}
@@ -58,13 +61,19 @@ std::vector<int> create_listen_sockets(const weaseldb::Config &config) {
if (bind(sfd, (struct sockaddr *)&addr, sizeof(addr)) == -1) {
perror("bind");
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
std::abort();
}
if (listen(sfd, SOMAXCONN) == -1) {
perror("listen");
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
std::abort();
}
@@ -103,7 +112,10 @@ std::vector<int> create_listen_sockets(const weaseldb::Config &config) {
int val = 1;
if (setsockopt(sfd, SOL_SOCKET, SO_REUSEADDR, &val, sizeof(val)) == -1) {
perror("setsockopt SO_REUSEADDR");
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
continue;
}
@@ -111,7 +123,10 @@ std::vector<int> create_listen_sockets(const weaseldb::Config &config) {
if (rp->ai_family == AF_INET || rp->ai_family == AF_INET6) {
if (setsockopt(sfd, IPPROTO_TCP, TCP_NODELAY, &val, sizeof(val)) == -1) {
perror("setsockopt TCP_NODELAY");
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
continue;
}
}
@@ -120,7 +135,10 @@ std::vector<int> create_listen_sockets(const weaseldb::Config &config) {
break; /* Success */
}
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
sfd = -1;
}
@@ -133,7 +151,10 @@ std::vector<int> create_listen_sockets(const weaseldb::Config &config) {
if (listen(sfd, SOMAXCONN) == -1) {
perror("listen");
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
std::abort();
}

34
src/server.cpp
View File

@@ -59,18 +59,27 @@ Server::Server(const weaseldb::Config &config, ConnectionHandler &handler,
Server::~Server() {
if (shutdown_pipe_[0] != -1) {
close(shutdown_pipe_[0]);
int e;
do {
e = close(shutdown_pipe_[0]);
} while (e == -1 && errno == EINTR);
shutdown_pipe_[0] = -1;
}
if (shutdown_pipe_[1] != -1) {
close(shutdown_pipe_[1]);
int e;
do {
e = close(shutdown_pipe_[1]);
} while (e == -1 && errno == EINTR);
shutdown_pipe_[1] = -1;
}
// Close all epoll instances
for (int epollfd : epoll_fds_) {
if (epollfd != -1) {
close(epollfd);
int e;
do {
e = close(epollfd);
} while (e == -1 && errno == EINTR);
}
}
epoll_fds_.clear();
@@ -78,7 +87,10 @@ Server::~Server() {
// Close all listen sockets (Server always owns them)
for (int fd : listen_fds_) {
if (fd != -1) {
close(fd);
int e;
do {
e = close(fd);
} while (e == -1 && errno == EINTR);
}
}
@@ -214,8 +226,13 @@ int Server::createLocalConnection() {
if (epoll_ctl(epollfd, EPOLL_CTL_ADD, server_fd, &event) == -1) {
perror("epoll_ctl ADD local connection");
connection_registry_.remove(server_fd);
close(server_fd);
close(client_fd);
int e;
do {
e = close(server_fd);
} while (e == -1 && errno == EINTR);
do {
e = close(client_fd);
} while (e == -1 && errno == EINTR);
return -1;
}
@@ -367,7 +384,10 @@ void Server::start_io_threads(std::vector<std::thread> &threads) {
if (config_.server.max_connections > 0 &&
active_connections_.load(std::memory_order_relaxed) >=
config_.server.max_connections) {
close(fd);
int e;
do {
e = close(fd);
} while (e == -1 && errno == EINTR);
continue;
}

35
style.md
View File

@@ -329,6 +329,41 @@ static_assert(std::is_trivially_destructible_v<T>, "Arena requires trivially des
// assert(file_exists(path)); // File might legitimately not exist - use return code instead
```
### System Call Error Handling
When a system call is interrupted by a signal (`EINTR`), it is usually necessary to retry the call. This is especially true for "slow" system calls that can block for a long time, such as `read`, `write`, `accept`, `connect`, `close`, `sem_wait`, and `epoll_wait`.
**Rule:** Always wrap potentially interruptible system calls in a `do-while` loop that checks for `EINTR`.
**Example:**
```cpp
int fd;
do {
fd = accept(listen_fd, nullptr, nullptr);
} while (fd == -1 && errno == EINTR);
if (fd == -1) {
// Handle other errors
perror("accept");
abort();
}
```
**Non-interruptible calls:**
Most system calls are not interruptible in practice. For these, it is not necessary to add a retry loop. This includes:
* `fcntl` (with `F_GETFL`, `F_SETFL`, `F_GETFD`, `F_SETFD` - note: `F_SETLKW` and `F_OFD_SETLKW` CAN return EINTR)
* `epoll_ctl`
* `socketpair`
* `pipe`
* `setsockopt`
* `epoll_create1`
When in doubt, consult the `man` page for the specific system call to see if it can return `EINTR`.
---
## Documentation

15
tests/test_server_connection_return.cpp
View File

@@ -69,12 +69,18 @@ TEST_CASE(
// Write some test data
const char *test_message = "Hello, World!";
ssize_t bytes_written = write(client_fd, test_message, strlen(test_message));
ssize_t bytes_written;
do {
bytes_written = write(client_fd, test_message, strlen(test_message));
} while (bytes_written == -1 && errno == EINTR);
REQUIRE(bytes_written == strlen(test_message));
// Read the echoed response
char buffer[1024] = {0};
ssize_t bytes_read = read(client_fd, buffer, sizeof(buffer) - 1);
ssize_t bytes_read;
do {
bytes_read = read(client_fd, buffer, sizeof(buffer) - 1);
} while (bytes_read == -1 && errno == EINTR);
if (bytes_read == -1) {
perror("read failed");
}
@@ -84,7 +90,10 @@ TEST_CASE(
CHECK(std::string(buffer, bytes_read) == std::string(test_message));
// Cleanup
close(client_fd);
int e;
do {
e = close(client_fd);
} while (e == -1 && errno == EINTR);
server->shutdown();
server_thread.join();
{

29
tools/load_tester.cpp
View File

@@ -85,11 +85,19 @@ int getConnectFd(const char *node, const char *service) {
continue;
}
if (connect(sfd, rp->ai_addr, rp->ai_addrlen) == 0) {
int conn_result;
do {
conn_result = connect(sfd, rp->ai_addr, rp->ai_addrlen);
} while (conn_result == -1 && errno == EINTR);
if (conn_result == 0) {
break; /* Success */
}
close(sfd);
int e;
do {
e = close(sfd);
} while (e == -1 && errno == EINTR);
}
freeaddrinfo(result); /* No longer needed */
@@ -113,7 +121,10 @@ int getConnectFdUnix(const char *socket_name) {
memset(&addr, 0, sizeof(addr));
addr.sun_family = AF_UNIX;
strncpy(addr.sun_path, socket_name, sizeof(addr.sun_path) - 1);
int e = connect(sfd, (struct sockaddr *)&addr, sizeof(addr));
int e;
do {
e = connect(sfd, (struct sockaddr *)&addr, sizeof(addr));
} while (e == -1 && errno == EINTR);
if (e == -1) {
perror("connect");
abort();
@@ -241,7 +252,10 @@ struct Connection {
bool error = false;
~Connection() {
int e = close(fd);
int e;
do {
e = close(fd);
} while (e == -1 && errno == EINTR);
if (e == -1) {
perror("close");
abort();
@@ -711,7 +725,9 @@ int main(int argc, char *argv[]) {
while (!g_shutdown.load(std::memory_order_relaxed)) {
int e;
{
do {
e = sem_wait(&connectionLimit);
} while (e == -1 && errno == EINTR);
if (e == -1) {
perror("sem_wait");
abort();
@@ -814,6 +830,9 @@ int main(int argc, char *argv[]) {
// Clean up epoll file descriptors
for (int epollfd : g_epoll_fds) {
close(epollfd);
int e;
do {
e = close(epollfd);
} while (e == -1 && errno == EINTR);
}
}