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

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());
 }
+