[Box Backup-commit] COMMIT r1458 - box/chris/general/lib/server

[boxbackup-dev at fluffy.co.uk]

Author: chris
Date: 2007-03-24 19:26:25 +0000 (Sat, 24 Mar 2007)
New Revision: 1458

Modified:
   box/chris/general/lib/server/WinNamedPipeStream.cpp
Log:
Improve logging of pipe errors by including the error message.

Reinitialise the OVERLAPPED structure each time we start a new overlapped
read (thanks Charles!)

Don't log errors or throw exceptions when we get ERROR_NO_DATA, which just 
means that the pipe is being closed. Treat it as a normal remote close
(EOF) instead.

Don't log an error if DisconnectNamedPipe tells us that the remote end
already closed the pipe (ERROR_PIPE_NOT_CONNECTED)


Modified: box/chris/general/lib/server/WinNamedPipeStream.cpp
===================================================================
--- box/chris/general/lib/server/WinNamedPipeStream.cpp	2007-03-24 19:23:16 UTC (rev 1457)
+++ box/chris/general/lib/server/WinNamedPipeStream.cpp	2007-03-24 19:26:25 UTC (rev 1458)
@@ -100,8 +100,8 @@
 
 	if (mSocketHandle == INVALID_HANDLE_VALUE)
 	{
-		::syslog(LOG_ERR, "CreateNamedPipeW failed: %d", 
-			GetLastError());
+		::syslog(LOG_ERR, "CreateNamedPipeW failed: %s", 
+			GetErrorMessage(GetLastError()).c_str());
 		THROW_EXCEPTION(ServerException, SocketOpenError)
 	}
 
@@ -109,8 +109,8 @@
 
 	if (!connected)
 	{
-		::syslog(LOG_ERR, "ConnectNamedPipe failed: %d", 
-			GetLastError());
+		::syslog(LOG_ERR, "ConnectNamedPipe failed: %s", 
+			GetErrorMessage(GetLastError()).c_str());
 		Close();
 		THROW_EXCEPTION(ServerException, SocketOpenError)
 	}
@@ -126,8 +126,8 @@
 
 	if (mReadableEvent == INVALID_HANDLE_VALUE)
 	{
-		::syslog(LOG_ERR, "Failed to create the Readable event: "
-			"error %d", GetLastError());
+		::syslog(LOG_ERR, "Failed to create the Readable event: %s",
+			GetErrorMessage(GetLastError()).c_str());
 		Close();
 		THROW_EXCEPTION(CommonException, Internal)
 	}
@@ -145,7 +145,7 @@
 		if (err != ERROR_IO_PENDING)
 		{
 			::syslog(LOG_ERR, "Failed to start overlapped read: "
-				"error %d", err);
+				"%s", GetErrorMessage(err).c_str());
 			Close();
 			THROW_EXCEPTION(ConnectionException, 
 				Conn_SocketReadError)
@@ -189,7 +189,7 @@
 		else
 		{
 			::syslog(LOG_ERR, "Failed to connect to backup "
-				"daemon: error %d", err);
+				"daemon: %s", GetErrorMessage(err).c_str());
 		}
 		THROW_EXCEPTION(ServerException, SocketOpenError)
 	}
@@ -269,7 +269,8 @@
 						::syslog(LOG_ERR, 
 							"Failed to wait for "
 							"ReadFile to complete: "
-							"error %d", err);
+							"%s", 
+							GetErrorMessage(err).c_str());
 					}
 
 					Close();
@@ -298,6 +299,13 @@
 		mBytesInBuffer = BytesRemaining;
 		NumBytesRead = BytesToCopy;
 
+		if (needAnotherRead)
+		{
+			// reinitialise the OVERLAPPED structure
+			memset(&mReadOverlap, 0, sizeof(mReadOverlap));
+			mReadOverlap.hEvent = mReadableEvent;
+		}
+
 		// start the next overlapped read
 		if (needAnotherRead && !ReadFile(mSocketHandle, 
 			mReadBuffer + mBytesInBuffer, 
@@ -325,7 +333,8 @@
 			else
 			{
 				::syslog(LOG_ERR, "Failed to start "
-					"overlapped read: error %d", err);
+					"overlapped read: %s", 
+					GetErrorMessage(err).c_str());
 				Close();
 				THROW_EXCEPTION(ConnectionException, 
 					Conn_SocketReadError)
@@ -364,9 +373,25 @@
 			&NumBytesRead, // number of bytes read 
 			NULL))         // not overlapped 
 		{
+			DWORD err = GetLastError();
+		
 			Close();
-			THROW_EXCEPTION(ConnectionException, 
-				Conn_SocketReadError)
+
+			// ERROR_NO_DATA is a strange name for 
+			// "The pipe is being closed". No exception wanted.
+
+			if (err == ERROR_NO_DATA) 
+			{
+				NumBytesRead = 0;
+			}
+			else
+			{
+				::syslog(LOG_ERR, "Failed to read from "
+					"control socket: %s", 
+					GetErrorMessage(err).c_str());
+				THROW_EXCEPTION(ConnectionException, 
+					Conn_SocketReadError)
+			}
 		}
 		
 		// Closed for reading at EOF?
@@ -413,9 +438,23 @@
 
 		if (!Success)
 		{
+			DWORD err = GetLastError();
+			::syslog(LOG_ERR, "Failed to write to control socket: "
+				"%s", GetErrorMessage(err).c_str());
 			Close();
-			THROW_EXCEPTION(ConnectionException, 
-				Conn_SocketWriteError)
+
+			// ERROR_NO_DATA is a strange name for 
+			// "The pipe is being closed". No exception wanted.
+
+			if (err == ERROR_NO_DATA) 
+			{
+				return;
+			}
+			else
+			{
+				THROW_EXCEPTION(ConnectionException, 
+					Conn_SocketWriteError)
+			}
 		}
 
 		NumBytesWrittenTotal += NumBytesWrittenThisTime;
@@ -449,7 +488,8 @@
 		if (!CancelIo(mSocketHandle))
 		{
 			::syslog(LOG_ERR, "Failed to cancel outstanding "
-				"I/O: error %d", GetLastError());
+				"I/O: %s", 
+				GetErrorMessage(GetLastError()).c_str());
 		}
 
 		if (mReadableEvent == INVALID_HANDLE_VALUE)
@@ -460,21 +500,27 @@
 		else if (!CloseHandle(mReadableEvent))
 		{
 			::syslog(LOG_ERR, "Failed to destroy Readable "
-				"event: error %d", GetLastError());
+				"event: %s", 
+				GetErrorMessage(GetLastError()).c_str());
 		}
 
 		mReadableEvent = INVALID_HANDLE_VALUE;
 
 		if (!FlushFileBuffers(mSocketHandle))
 		{
-			::syslog(LOG_INFO, "FlushFileBuffers failed: %d", 
-				GetLastError());
+			::syslog(LOG_INFO, "FlushFileBuffers failed: %s", 
+				GetErrorMessage(GetLastError()).c_str());
 		}
 	
 		if (!DisconnectNamedPipe(mSocketHandle))
 		{
-			::syslog(LOG_ERR, "DisconnectNamedPipe failed: %d", 
-				GetLastError());
+			DWORD err = GetLastError();
+			if (err != ERROR_PIPE_NOT_CONNECTED)
+			{
+				::syslog(LOG_ERR, "DisconnectNamedPipe "
+					"failed: %s", 
+					GetErrorMessage(err).c_str());
+			}
 		}
 
 		mIsServer = false;
@@ -489,7 +535,8 @@
 
 	if (!result) 
 	{
-		::syslog(LOG_ERR, "CloseHandle failed: %d", GetLastError());
+		::syslog(LOG_ERR, "CloseHandle failed: %s", 
+			GetErrorMessage(GetLastError()).c_str());
 		THROW_EXCEPTION(ServerException, SocketCloseError)
 	}
 }
@@ -537,8 +584,8 @@
 	
 	if (!FlushFileBuffers(mSocketHandle))
 	{
-		::syslog(LOG_WARNING, "FlushFileBuffers failed: %d", 
-			GetLastError());
+		::syslog(LOG_WARNING, "FlushFileBuffers failed: %s", 
+			GetErrorMessage(GetLastError()).c_str());
 	}
 }