Remove Win32 command socket thread, as it has caused too much trouble.

Handle command socket on Win32 the same as all other platforms, removing
#ifdefs from BackupDaemon.

Will replace this thread with regular but not excessive command socket
polling using timers in future.

Change error messages when command socket comms fail to make them clearer.

2 files changed, 86 insertions, 63 deletions

diff --git a/lib/server/WinNamedPipeStream.cpp b/lib/server/WinNamedPipeStream.cpp
index b3829d6b..1179516e 100644
--- a/lib/server/WinNamedPipeStream.cpp
+++ b/lib/server/WinNamedPipeStream.cpp
@@ -44,7 +44,55 @@ WinNamedPipeStream::WinNamedPipeStream()
 	  mWriteClosed(false),
 	  mIsServer(false),
 	  mIsConnected(false)
-{
+{ }
+
+// --------------------------------------------------------------------------
+//
+// Function
+//		Name:    WinNamedPipeStream::WinNamedPipeStream(HANDLE)
+//		Purpose: Constructor (with already-connected pipe handle)
+//		Created: 2008/10/01
+//
+// --------------------------------------------------------------------------
+WinNamedPipeStream::WinNamedPipeStream(HANDLE hNamedPipe)
+	: mSocketHandle(hNamedPipe),
+	  mReadableEvent(INVALID_HANDLE_VALUE),
+	  mBytesInBuffer(0),
+	  mReadClosed(false),
+	  mWriteClosed(false),
+	  mIsServer(true),
+	  mIsConnected(true)
+{ 
+	// create the Readable event
+	mReadableEvent = CreateEvent(NULL, TRUE, FALSE, NULL);
+
+	if (mReadableEvent == INVALID_HANDLE_VALUE)
+	{
+		BOX_ERROR("Failed to create the Readable event: " <<
+			GetErrorMessage(GetLastError()));
+		Close();
+		THROW_EXCEPTION(CommonException, Internal)
+	}
+
+	// initialise the OVERLAPPED structure
+	memset(&mReadOverlap, 0, sizeof(mReadOverlap));
+	mReadOverlap.hEvent = mReadableEvent;
+
+	// start the first overlapped read
+	if (!ReadFile(mSocketHandle, mReadBuffer, sizeof(mReadBuffer),
+		NULL, &mReadOverlap))
+	{
+		DWORD err = GetLastError();
+
+		if (err != ERROR_IO_PENDING)
+		{
+			BOX_ERROR("Failed to start overlapped read: " <<
+				GetErrorMessage(err));
+			Close();
+			THROW_EXCEPTION(ConnectionException, 
+				Conn_SocketReadError)
+		}
+	}
 }
 
 // --------------------------------------------------------------------------
@@ -80,33 +128,17 @@ WinNamedPipeStream::~WinNamedPipeStream()
 //		Created: 2005/12/07
 //
 // --------------------------------------------------------------------------
-void WinNamedPipeStream::Accept(const std::string& rName)
+/*
+void WinNamedPipeStream::Accept()
 {
-	if (mSocketHandle != INVALID_HANDLE_VALUE || mIsConnected) 
+	if (mSocketHandle == INVALID_HANDLE_VALUE)
 	{
-		THROW_EXCEPTION(ServerException, SocketAlreadyOpen)
+		THROW_EXCEPTION(ServerException, BadSocketHandle);
 	}
 
-	std::string socket = sPipeNamePrefix + rName;
-
-	mSocketHandle = CreateNamedPipeA( 
-		socket.c_str(),            // pipe name 
-		PIPE_ACCESS_DUPLEX |       // read/write access 
-		FILE_FLAG_OVERLAPPED,      // enabled overlapped I/O
-		PIPE_TYPE_BYTE |           // message type pipe 
-		PIPE_READMODE_BYTE |       // message-read mode 
-		PIPE_WAIT,                 // blocking mode 
-		1,                         // max. instances  
-		4096,                      // output buffer size 
-		4096,                      // input buffer size 
-		NMPWAIT_USE_DEFAULT_WAIT,  // client time-out 
-		NULL);                     // default security attribute 
-
-	if (mSocketHandle == INVALID_HANDLE_VALUE)
+	if (mIsConnected) 
 	{
-		BOX_ERROR("Failed to CreateNamedPipeA(" << socket << "): " <<
-			GetErrorMessage(GetLastError()));
-		THROW_EXCEPTION(ServerException, SocketOpenError)
+		THROW_EXCEPTION(ServerException, SocketAlreadyOpen);
 	}
 
 	bool connected = ConnectNamedPipe(mSocketHandle, (LPOVERLAPPED) NULL);
@@ -156,6 +188,7 @@ void WinNamedPipeStream::Accept(const std::string& rName)
 		}
 	}
 }
+*/
 
 // --------------------------------------------------------------------------
 //
@@ -217,7 +250,7 @@ void WinNamedPipeStream::Connect(const std::string& rName)
 int WinNamedPipeStream::Read(void *pBuffer, int NBytes, int Timeout)
 {
 	// TODO no support for timeouts yet
-	if (Timeout != IOStream::TimeOutInfinite)
+	if (!mIsServer && Timeout != IOStream::TimeOutInfinite)
 	{
 		THROW_EXCEPTION(CommonException, AssertFailed)
 	}
@@ -249,8 +282,29 @@ int WinNamedPipeStream::Read(void *pBuffer, int NBytes, int Timeout)
 		{
 			// overlapped I/O completed successfully? 
 			// (wait if needed)
+			DWORD waitResult = WaitForSingleObject(
+				mReadOverlap.hEvent, Timeout);
 
-			if (GetOverlappedResult(mSocketHandle,
+			if (waitResult == WAIT_ABANDONED)
+			{
+				BOX_ERROR("Wait for command socket read "
+					"abandoned by system");
+				THROW_EXCEPTION(ServerException,
+					BadSocketHandle);
+			}
+			else if (waitResult == WAIT_TIMEOUT)
+			{
+				// wait timed out, nothing to read
+				NumBytesRead = 0;
+			}
+			else if (waitResult != WAIT_OBJECT_0)
+			{
+				BOX_ERROR("Failed to wait for command "
+					"socket read: unknown result " <<
+					waitResult);
+			}
+			// object is ready to read from
+			else if (GetOverlappedResult(mSocketHandle,
 				&mReadOverlap, &NumBytesRead, TRUE))
 			{
 				needAnotherRead = true;
@@ -267,7 +321,7 @@ int WinNamedPipeStream::Read(void *pBuffer, int NBytes, int Timeout)
 				{
 					if (err == ERROR_BROKEN_PIPE)
 					{
-						BOX_ERROR("Control client "
+						BOX_NOTICE("Control client "
 							"disconnected");
 					}
 					else
@@ -342,29 +396,6 @@ int WinNamedPipeStream::Read(void *pBuffer, int NBytes, int Timeout)
 					Conn_SocketReadError)
 			}
 		}
-
-		// 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
 	{
@@ -441,7 +472,7 @@ void WinNamedPipeStream::Write(const void *pBuffer, int NBytes)
 		if (!Success)
 		{
 			// ERROR_NO_DATA is a strange name for 
-			// "The pipe is being closed". No exception wanted.
+			// "The pipe is being closed".
 
 			DWORD err = GetLastError();
 
@@ -453,15 +484,8 @@ void WinNamedPipeStream::Write(const void *pBuffer, int NBytes)
 
 			Close();
 
-			if (err == ERROR_NO_DATA) 
-			{
-				return;
-			}
-			else
-			{
-				THROW_EXCEPTION(ConnectionException, 
-					Conn_SocketWriteError)
-			}
+			THROW_EXCEPTION(ConnectionException, 
+				Conn_SocketWriteError)
 		}
 
 		NumBytesWrittenTotal += NumBytesWrittenThisTime;
diff --git a/lib/server/WinNamedPipeStream.h b/lib/server/WinNamedPipeStream.h
index 6acd48f6..386ff7e3 100644
--- a/lib/server/WinNamedPipeStream.h
+++ b/lib/server/WinNamedPipeStream.h
@@ -24,10 +24,11 @@ class WinNamedPipeStream : public IOStream
 {
 public:
 	WinNamedPipeStream();
+	WinNamedPipeStream(HANDLE hNamedPipe);
 	~WinNamedPipeStream();
 
 	// server side - create the named pipe and listen for connections
-	void Accept(const std::string& rName);
+	// use WinNamedPipeListener to do this instead.
 
 	// client side - connect to a waiting server
 	void Connect(const std::string& rName);
@@ -40,9 +41,6 @@ public:
 	virtual void Close();
 	virtual bool StreamDataLeft();
 	virtual bool StreamClosed();
-	bool IsConnected() { return mIsConnected; }
-	HANDLE GetSocketHandle() { return mSocketHandle; }
-	HANDLE GetReadableEvent() { return mReadableEvent; }
 
 protected:
 	void MarkAsReadClosed()  {mReadClosed  = true;}
@@ -62,6 +60,7 @@ private:
 	bool mIsServer;
 	bool mIsConnected;
 
+public:
 	static std::string sPipeNamePrefix;
 };