From 1a59b542f90c10799085ad2b2d8aed2a6c378acf Mon Sep 17 00:00:00 2001 From: brain Date: Thu, 14 Dec 2006 17:58:13 +0000 Subject: Change how users are quit if they get a write error during the things they do. Instead of QuitUser inside FlushWriteBuffer() (potentially *UNSAFE*), go back to using SetWriteError(), but to ensure we dont get a cascade of bad write events from the socket engine, QuitUser the user before returning in userrec::HandleEvent, after we can gaurantee ALL other reading or writing is done. git-svn-id: http://svn.inspircd.org/repository/trunk/inspircd@5987 e03df62e-2008-0410-955e-edbf42e46eb7 --- src/userprocess.cpp | 1 - src/users.cpp | 20 +++++++++++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/userprocess.cpp b/src/userprocess.cpp index 43edb56b1..9aa6d4a31 100644 --- a/src/userprocess.cpp +++ b/src/userprocess.cpp @@ -331,7 +331,6 @@ void InspIRCd::DoBackgroundUserStuff(time_t TIME) * We can flush the write buffer as the last thing we do, because if they * match any of the above conditions its no use flushing their buffer anyway. */ - curr->FlushWriteBuf(); } } diff --git a/src/users.cpp b/src/users.cpp index 4bfb8955a..2f1916391 100644 --- a/src/users.cpp +++ b/src/users.cpp @@ -667,21 +667,26 @@ void userrec::FlushWriteBuf() { if (errno == EAGAIN) { + /* The socket buffer is full. This isnt fatal, + * try again later. + */ ServerInstance->Log(DEBUG,"EAGAIN, want write"); this->ServerInstance->SE->WantWrite(this); } else { - this->QuitUser(ServerInstance, this, strerror(errno)); + /* Fatal error, set write error and bail + */ + this->SetWriteError(strerror(errno)); return; } } else { - // advance the queue + /* advance the queue */ if (n_sent) this->sendq = this->sendq.substr(n_sent); - // update the user's stats counters + /* update the user's stats counters */ this->bytes_out += n_sent; this->cmds_out++; if (n_sent != old_sendq_length) @@ -703,7 +708,6 @@ void userrec::SetWriteError(const std::string &error) { try { - ServerInstance->Log(DEBUG,"SetWriteError: %s",error.c_str()); // don't try to set the error twice, its already set take the first string. if (this->WriteError.empty()) { @@ -2031,13 +2035,14 @@ void userrec::HandleEvent(EventType et, int errornum) { case EVENT_READ: ServerInstance->ProcessUser(this); + break; case EVENT_WRITE: this->FlushWriteBuf(); break; case EVENT_ERROR: /** This should be safe, but dont DARE do anything after it -- Brain */ - userrec::QuitUser(ServerInstance, this, errornum ? strerror(errornum) : "EOF from client"); + this->SetWriteError(errornum ? strerror(errornum) : "EOF from client"); break; } } @@ -2045,4 +2050,9 @@ void userrec::HandleEvent(EventType et, int errornum) { ServerInstance->Log(DEBUG,"Exception in userrec::HandleEvent intercepted"); } + + /* If the user has raised an error whilst being processed, quit them now we're safe to */ + if (!WriteError.empty()) + userrec::QuitUser(ServerInstance, this, GetWriteError()); } + -- cgit v1.2.3