[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