From 28263a28c05a1fe676804d499634e3d364c2da3b Mon Sep 17 00:00:00 2001 From: ArchCipher Date: Mon, 21 Sep 2026 17:08:48 +0200 Subject: [PATCH 1/2] fix: pollout before send() --- includes/Server.hpp | 1 + includes/Socket.hpp | 2 ++ srcs/server/Server.cpp | 19 +++++++++++++++++++ srcs/server/Socket.cpp | 43 ++++++++++++++++++++++++++++-------------- 4 files changed, 51 insertions(+), 14 deletions(-) diff --git a/includes/Server.hpp b/includes/Server.hpp index 931ed4c..c16ecde 100644 --- a/includes/Server.hpp +++ b/includes/Server.hpp @@ -62,6 +62,7 @@ class Server { void removeClientSocketPfd_(int const fd, std::string const &msg); void storePollFd_(int const socketFd, bool srvSocket); void removePollFd_(int const fd); + void setPolloutInterest_(int const fd); void log_(std::string const &msg, Logger::logMode const lm); Server(Server const &other); diff --git a/includes/Socket.hpp b/includes/Socket.hpp index e208f71..707e042 100644 --- a/includes/Socket.hpp +++ b/includes/Socket.hpp @@ -43,6 +43,8 @@ class Socket { bool checkSocketData(int const fd); std::string getCRLFLineFromBuffer(int const fd, bool &buffHasMoreCmds, bool &msgTooLong); + bool hasPendingWrite(int const fd); + bool flushWriteBuffer(int const fd); void sendMsg(int const fd, std::string const &msg); bool socketCloseMarker(int const fd); void closeSocket(int const fd); diff --git a/srcs/server/Server.cpp b/srcs/server/Server.cpp index d5baca3..bc9fb69 100644 --- a/srcs/server/Server.cpp +++ b/srcs/server/Server.cpp @@ -133,6 +133,10 @@ void Server::processClientSockets_() { continue; } + if (pfd.revents & (POLLOUT)) + sockets_.flushWriteBuffer(pfd.fd); + setPolloutInterest_(pfd.fd); + if (!(pfd.revents & (POLLIN | POLLERR | POLLHUP))) continue; @@ -211,6 +215,10 @@ void Server::removeClientSocketPfd_(int const fd, std::string const &msg) { } clients_.remove(fd, channels_); } + // TODO: socket is closed without flush. So loop cannot send the message + // Fix line below : + // sockets_.flushWriteBuffer(fd); + // But, is it okay to flush without checking POLLOUT here? sockets_.closeSocket(fd); removePollFd_(fd); } @@ -343,6 +351,17 @@ void Server::sendMsg(int const fd, std::string const &msg) { logger_.logMsg(oss.str(), Logger::lmOut); } sockets_.sendMsg(fd, msg); + setPolloutInterest_(fd); +} + +void Server::setPolloutInterest_(int const fd) { + std::vector::iterator it = + std::find_if(pollFds_.begin(), pollFds_.end(), srvHelp::PfdHasFd(fd)); + if (it == pollFds_.end()) + return; + short const pollout = static_cast(POLLOUT); + short const interest = sockets_.hasPendingWrite(fd) ? pollout : 0; + it->events = static_cast((it->events & ~pollout) | interest); } #ifdef BONUS diff --git a/srcs/server/Socket.cpp b/srcs/server/Socket.cpp index d7dd603..db19a3b 100644 --- a/srcs/server/Socket.cpp +++ b/srcs/server/Socket.cpp @@ -121,10 +121,37 @@ size_t Socket::bufferLengthAndSanityCheck_(SocketIOData &sIOD, bool &msgTooLong, return std::string::npos; } +bool Socket::hasPendingWrite(int const fd) { + SocketIOData *sIODptr = getSockIOData(fd); + return sIODptr && !sIODptr->closeFd && !sIODptr->writeBuffer.empty(); +} + +// return true if nothing to write +// returns false on error +bool Socket::flushWriteBuffer(int const fd) { + SocketIOData *sIODptr = getSockIOData(fd); + if (!sIODptr) + return true; + SocketIOData &sIOD = *sIODptr; + while (!sIOD.writeBuffer.empty() && !sIOD.closeFd) { + ssize_t bytesSent = send(fd, sIOD.writeBuffer.c_str(), + sIOD.writeBuffer.length(), MSG_NOSIGNAL); + if (bytesSent > 0) { + sIOD.writeBuffer.erase(0, static_cast(bytesSent)); + continue; + } + // No errror count to make sure slow clients are not killed. + // Errors EAGAIN/EWOULDBLOCK are acceptable + return false; + } + return sIOD.writeBuffer.empty(); +} + /* sendMsg * send string msg to a network socket * uses i/o buffers and error counters in SocketIOData + * only queue no send. sent in poll loop after checking POLLOUT */ void Socket::sendMsg(int const fd, std::string const &msg) { SocketIOData *sIODptr = getSockIOData(fd); @@ -132,26 +159,14 @@ void Socket::sendMsg(int const fd, std::string const &msg) { return; SocketIOData &sIOD = *sIODptr; + if (sIOD.closeFd) + return; if (msg.length() > sIOD.writeBuffer.max_size() - sIOD.writeBuffer.length()) { sIOD.closeFd = true; return; } sIOD.writeBuffer += msg; - ssize_t bytesSent = send(fd, sIOD.writeBuffer.c_str(), - sIOD.writeBuffer.length(), MSG_NOSIGNAL); - if (bytesSent <= 0) { - ++sIOD.sendErrorCount; - if (sIOD.sendErrorCount >= SOCKETERRORLIMIT) - sIOD.closeFd = true; - return; - } - - sIOD.sendErrorCount = 0; - if (static_cast(bytesSent) < sIOD.writeBuffer.length()) - sIOD.writeBuffer.erase(0, bytesSent); - else - sIOD.writeBuffer.clear(); } bool Socket::socketCloseMarker(int const fd) { From a35aab5fceac700c2a06cdf9e55ff2e138badafe Mon Sep 17 00:00:00 2001 From: ArchCipher Date: Mon, 21 Sep 2026 17:51:49 +0200 Subject: [PATCH 2/2] chore: format --- srcs/server/Server.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/srcs/server/Server.cpp b/srcs/server/Server.cpp index bc9fb69..a7121b1 100644 --- a/srcs/server/Server.cpp +++ b/srcs/server/Server.cpp @@ -216,9 +216,9 @@ void Server::removeClientSocketPfd_(int const fd, std::string const &msg) { clients_.remove(fd, channels_); } // TODO: socket is closed without flush. So loop cannot send the message - // Fix line below : - // sockets_.flushWriteBuffer(fd); - // But, is it okay to flush without checking POLLOUT here? + // Fix line below : + // sockets_.flushWriteBuffer(fd); + // But, is it okay to flush without checking POLLOUT here? sockets_.closeSocket(fd); removePollFd_(fd); }