[Box Backup-dev] COMMIT r643 - box/chris/general/lib/server

boxbackup-dev at fluffy.co.uk boxbackup-dev at fluffy.co.uk
Mon Jul 10 21:54:28 BST 2006


Author: chris
Date: 2006-07-10 20:54:27 +0000 (Mon, 10 Jul 2006)
New Revision: 643

Modified:
   box/chris/general/lib/server/WinNamedPipeStream.cpp
Log:
* WinNamedPipeStream.cpp
- Throw exception if Read() called on a closed stream (edge case?)
- Satisfy reads from buffer if possible to avoid problems with small read
  buffers in debug builds on Win32
- May still have a problem with the readable event being signalled multiple
  times before reset, resulting in lost events and reader thread and
  control client hanging.


Modified: box/chris/general/lib/server/WinNamedPipeStream.cpp
===================================================================
--- box/chris/general/lib/server/WinNamedPipeStream.cpp	2006-07-10 20:49:02 UTC (rev 642)
+++ box/chris/general/lib/server/WinNamedPipeStream.cpp	2006-07-10 20:54:27 UTC (rev 643)
@@ -212,36 +212,61 @@
 		THROW_EXCEPTION(ServerException, BadSocketHandle)
 	}
 
+	if (mReadClosed)
+	{
+		THROW_EXCEPTION(ConnectionException, SocketShutdownError)
+	}
+
 	DWORD NumBytesRead;
 
 	if (mIsServer)
 	{
-		// overlapped I/O completed successfully? (wait if needed)
-
-		if (!GetOverlappedResult(mSocketHandle,
-			&mReadOverlap, &NumBytesRead, TRUE))
+		// satisfy from buffer if possible, to avoid
+		// blocking on read.
+		bool needAnotherRead = false;
+		if (mBytesInBuffer == 0)
 		{
-			DWORD err = GetLastError();
+			// overlapped I/O completed successfully? 
+			// (wait if needed)
 
-			if (err == ERROR_HANDLE_EOF)
+			if (GetOverlappedResult(mSocketHandle,
+				&mReadOverlap, &NumBytesRead, TRUE))
 			{
-				mReadClosed = true;
+				needAnotherRead = true;
 			}
-			else if (err == ERROR_BROKEN_PIPE)
-			{
-				::syslog(LOG_ERR, 
-					"Control client disconnected");
-				mReadClosed = true;
-			}
 			else
 			{
-				::syslog(LOG_ERR, "Failed to wait for "
-					"ReadFile to complete: error %d", err);
-				Close();
-				THROW_EXCEPTION(ConnectionException, 
-					Conn_SocketReadError)
+				DWORD err = GetLastError();
+
+				if (err == ERROR_HANDLE_EOF)
+				{
+					mReadClosed = true;
+				}
+				else 
+				{
+					if (err == ERROR_BROKEN_PIPE)
+					{
+						::syslog(LOG_ERR, "Control "
+							"client disconnected");
+					}
+					else
+					{
+						::syslog(LOG_ERR, 
+							"Failed to wait for "
+							"ReadFile to complete: "
+							"error %d", err);
+					}
+
+					Close();
+					THROW_EXCEPTION(ConnectionException, 
+						Conn_SocketReadError)
+				}
 			}
 		}
+		else
+		{
+			NumBytesRead = 0;
+		}
 
 		size_t BytesToCopy = NumBytesRead + mBytesInBuffer;
 		size_t BytesRemaining = 0;
@@ -259,7 +284,7 @@
 		NumBytesRead = BytesToCopy;
 
 		// start the next overlapped read
-		if (!ReadFile(mSocketHandle, 
+		if (needAnotherRead && !ReadFile(mSocketHandle, 
 			mReadBuffer + mBytesInBuffer, 
 			sizeof(mReadBuffer) - mBytesInBuffer,
 			NULL, &mReadOverlap))
@@ -267,12 +292,21 @@
 			DWORD err = GetLastError();
 			if (err == ERROR_IO_PENDING)
 			{
-				ResetEvent(mReadableEvent);
+				// Don't reset yet, there might be data
+				// in the buffer waiting to be read, 
+				// will check below.
+				// ResetEvent(mReadableEvent);
 			}
 			else if (err == ERROR_HANDLE_EOF)
 			{
 				mReadClosed = true;
 			}
+			else if (err == ERROR_BROKEN_PIPE)
+			{
+				::syslog(LOG_ERR, 
+					"Control client disconnected");
+				mReadClosed = true;
+			}
 			else
 			{
 				::syslog(LOG_ERR, "Failed to start "
@@ -286,6 +320,25 @@
 		// If the read succeeded immediately, leave the event 
 		// signaled, so that we will be called again to process 
 		// the newly read data and start another overlapped read.
+		if (needAnotherRead && !mReadClosed)
+		{
+			// leave signalled
+		}
+		else if (!needAnotherRead && mBytesInBuffer > 0)
+		{
+			// leave signalled
+		}
+		else
+		{
+			// nothing left to read, reset the event
+			ResetEvent(mReadableEvent);
+			// FIXME: a pending read could have signalled
+			// the event (again) while we were busy reading.
+			// that signal would be lost, and the reading
+			// thread would block. Should be pretty obvious
+			// if this happens in practice: control client
+			// hangs.
+		}
 	}
 	else
 	{




More information about the Boxbackup-dev mailing list