Fix deadlock waiting for read or write on closed connection.

If the system is suspended then it may not realise that a TCP connection has
been closed, while waiting for data to arrive on it. We didn't used to apply
a timeout to this read operation. Now we use the connection's default timeout
on all read and write operations. Network operations that don't pass a timeout
will be logged with a backtrace, so that they can be fixed.

6 files changed, 98 insertions, 67 deletions

diff --git a/lib/server/ProtocolUncertainStream.cpp b/lib/server/ProtocolUncertainStream.cpp
index 336fd36d..aeb15816 100644
--- a/lib/server/ProtocolUncertainStream.cpp
+++ b/lib/server/ProtocolUncertainStream.cpp
@@ -173,7 +173,7 @@ IOStream::pos_type ProtocolUncertainStream::BytesLeftToRead()
 //		Created: 2003/12/05
 //
 // --------------------------------------------------------------------------
-void ProtocolUncertainStream::Write(const void *pBuffer, int NBytes)
+void ProtocolUncertainStream::Write(const void *pBuffer, int NBytes, int Timeout)
 {
 	THROW_EXCEPTION(ServerException, CantWriteToProtocolUncertainStream)
 }
diff --git a/lib/server/ProtocolUncertainStream.h b/lib/server/ProtocolUncertainStream.h
index 4954cf88..2e97ba6a 100644
--- a/lib/server/ProtocolUncertainStream.h
+++ b/lib/server/ProtocolUncertainStream.h
@@ -33,7 +33,8 @@ private:
 public:
 	virtual int Read(void *pBuffer, int NBytes, int Timeout = IOStream::TimeOutInfinite);
 	virtual pos_type BytesLeftToRead();
-	virtual void Write(const void *pBuffer, int NBytes);
+	virtual void Write(const void *pBuffer, int NBytes,
+		int Timeout = IOStream::TimeOutInfinite);
 	virtual bool StreamDataLeft();
 	virtual bool StreamClosed();
 
diff --git a/lib/server/SocketStream.cpp b/lib/server/SocketStream.cpp
index 7b46b024..dafb4338 100644
--- a/lib/server/SocketStream.cpp
+++ b/lib/server/SocketStream.cpp
@@ -34,6 +34,7 @@
 #include "SocketStream.h"
 #include "CommonException.h"
 #include "Socket.h"
+#include "Utils.h"
 
 #include "MemLeakFindOn.h"
 
@@ -204,7 +205,9 @@ void SocketStream::Open(Socket::Type Type, const std::string& rName, int Port)
 // --------------------------------------------------------------------------
 int SocketStream::Read(void *pBuffer, int NBytes, int Timeout)
 {
-	if(mSocketHandle == INVALID_SOCKET_VALUE) 
+	CheckForMissingTimeout(Timeout);
+
+	if(mSocketHandle == INVALID_SOCKET_VALUE)
 	{
 		THROW_EXCEPTION(ServerException, BadSocketHandle)
 	}
@@ -215,7 +218,7 @@ int SocketStream::Read(void *pBuffer, int NBytes, int Timeout)
 		p.fd = mSocketHandle;
 		p.events = POLLIN;
 		p.revents = 0;
-		switch(::poll(&p, 1, (Timeout == IOStream::TimeOutInfinite)?INFTIM:Timeout))
+		switch(::poll(&p, 1, PollTimeout(Timeout)))
 		{
 		case -1:
 			// error
@@ -275,6 +278,43 @@ int SocketStream::Read(void *pBuffer, int NBytes, int Timeout)
 	return r;
 }
 
+bool SocketStream::Poll(short Events, int Timeout)
+{
+	// Wait for data to send.
+	struct pollfd p;
+	p.fd = GetSocketHandle();
+	p.events = Events;
+	p.revents = 0;
+
+	box_time_t start = GetCurrentBoxTime();
+	box_time_t end   = start + MilliSecondsToBoxTime(Timeout);
+	int result;
+
+	do
+	{
+		box_time_t now = GetCurrentBoxTime();
+		result = ::poll(&p, 1, PollTimeout(end - now));
+	}
+	while(result == -1 && errno == EINTR);
+
+	switch(result)
+	{
+	case -1:
+		// error - Bad!
+		THROW_SYS_ERROR("Failed to poll socket", ServerException,
+			SocketPollError);
+		break;
+
+	case 0:
+		// Condition not met, timed out
+		return false;
+
+	default:
+		// good to go!
+		return true;
+	}
+}
+
 // --------------------------------------------------------------------------
 //
 // Function
@@ -283,7 +323,7 @@ int SocketStream::Read(void *pBuffer, int NBytes, int Timeout)
 //		Created: 2003/07/31
 //
 // --------------------------------------------------------------------------
-void SocketStream::Write(const void *pBuffer, int NBytes)
+void SocketStream::Write(const void *pBuffer, int NBytes, int Timeout)
 {
 	if(mSocketHandle == INVALID_SOCKET_VALUE) 
 	{
@@ -296,7 +336,9 @@ void SocketStream::Write(const void *pBuffer, int NBytes)
 	
 	// Bytes left to send
 	int bytesLeft = NBytes;
-	
+	box_time_t start = GetCurrentBoxTime();
+	box_time_t end   = start + MilliSecondsToBoxTime(Timeout);
+
 	while(bytesLeft > 0)
 	{
 		// Try to send.
@@ -327,23 +369,15 @@ void SocketStream::Write(const void *pBuffer, int NBytes)
 			BOX_TRACE("Waiting to send data on socket " << 
 				mSocketHandle << " (" << bytesLeft <<
 				" of " << NBytes << " bytes left)");
-			
-			// Wait for data to send.
-			struct pollfd p;
-			p.fd = mSocketHandle;
-			p.events = POLLOUT;
-			p.revents = 0;
-			
-			if(::poll(&p, 1, 16000 /* 16 seconds */) == -1)
+
+			box_time_t now = GetCurrentBoxTime();
+
+			if(!Poll(POLLOUT, PollTimeout(end - now)))
 			{
-				// Don't exception if it's just a signal
-				if(errno != EINTR)
-				{
-					BOX_LOG_SYS_ERROR("Failed to poll "
-						"socket");
-					THROW_EXCEPTION(ServerException,
-						SocketPollError)
-				}
+				THROW_EXCEPTION_MESSAGE(ConnectionException,
+					Protocol_Timeout, "Timed out waiting "
+					"to send " << bytesLeft << " of " <<
+					NBytes << " bytes");
 			}
 		}
 	}
@@ -514,3 +548,11 @@ bool SocketStream::GetPeerCredentials(uid_t &rUidOut, gid_t &rGidOut)
 	return false;
 }
 
+void SocketStream::CheckForMissingTimeout(int Timeout)
+{
+	if (Timeout == IOStream::TimeOutInfinite)
+	{
+		BOX_WARNING("Network operation started with no timeout!");
+		DumpStackBacktrace();
+	}
+}
diff --git a/lib/server/SocketStream.h b/lib/server/SocketStream.h
index 2fb5e391..406d29e4 100644
--- a/lib/server/SocketStream.h
+++ b/lib/server/SocketStream.h
@@ -10,6 +10,9 @@
 #ifndef SOCKETSTREAM__H
 #define SOCKETSTREAM__H
 
+#include <climits>
+
+#include "BoxTime.h"
 #include "IOStream.h"
 #include "Socket.h"
 
@@ -41,7 +44,8 @@ public:
 	void Attach(int socket);
 
 	virtual int Read(void *pBuffer, int NBytes, int Timeout = IOStream::TimeOutInfinite);
-	virtual void Write(const void *pBuffer, int NBytes);
+	virtual void Write(const void *pBuffer, int NBytes,
+		int Timeout = IOStream::TimeOutInfinite);
 	virtual void Close();
 	virtual bool StreamDataLeft();
 	virtual bool StreamClosed();
@@ -53,6 +57,24 @@ public:
 protected:
 	void MarkAsReadClosed() {mReadClosed = true;}
 	void MarkAsWriteClosed() {mWriteClosed = true;}
+	void CheckForMissingTimeout(int Timeout);
+
+	int PollTimeout(box_time_t Timeout)
+	{
+		if (Timeout < 0)
+		{
+			return 0;
+		}
+		else if (Timeout == IOStream::TimeOutInfinite || Timeout > INT_MAX)
+		{
+			return INFTIM;
+		}
+		else
+		{
+			return (int) Timeout;
+		}
+	}
+	bool Poll(short Events, int Timeout);
 
 private:
 	tOSSocketHandle mSocketHandle;
diff --git a/lib/server/SocketStreamTLS.cpp b/lib/server/SocketStreamTLS.cpp
index 127d697a..6ca172f6 100644
--- a/lib/server/SocketStreamTLS.cpp
+++ b/lib/server/SocketStreamTLS.cpp
@@ -230,16 +230,17 @@ void SocketStreamTLS::Handshake(const TLSContext &rContext, bool IsServer)
 // --------------------------------------------------------------------------
 bool SocketStreamTLS::WaitWhenRetryRequired(int SSLErrorCode, int Timeout)
 {
-	struct pollfd p;
-	p.fd = GetSocketHandle();
+	CheckForMissingTimeout(Timeout);
+
+	short events;
 	switch(SSLErrorCode)
 	{
 	case SSL_ERROR_WANT_READ:
-		p.events = POLLIN;
+		events = POLLIN;
 		break;
 		
 	case SSL_ERROR_WANT_WRITE:
-		p.events = POLLOUT;
+		events = POLLOUT;
 		break;
 
 	default:
@@ -247,45 +248,8 @@ bool SocketStreamTLS::WaitWhenRetryRequired(int SSLErrorCode, int Timeout)
 		THROW_EXCEPTION(ServerException, Internal)
 		break;
 	}
-	p.revents = 0;
-
-	int64_t start, end;
-	start = BoxTimeToMilliSeconds(GetCurrentBoxTime());
-	end   = start + Timeout;
-	int result;
-
-	do
-	{
-		int64_t now = BoxTimeToMilliSeconds(GetCurrentBoxTime());
-		int poll_timeout = (int)(end - now);
-		if (poll_timeout < 0) poll_timeout = 0;
-		if (Timeout == IOStream::TimeOutInfinite)
-		{
-			poll_timeout = INFTIM;
-		}
-		result = ::poll(&p, 1, poll_timeout);
-	}
-	while(result == -1 && errno == EINTR);
-
-	switch(result)
-	{
-	case -1:
-		// error - Bad!
-		THROW_EXCEPTION(ServerException, SocketPollError)
-		break;
-
-	case 0:
-		// Condition not met, timed out
-		return false;
-		break;
-
-	default:
-		// good to go!
-		return true;
-		break;
-	}
 
-	return true;
+	return Poll(events, Timeout);
 }
 
 // --------------------------------------------------------------------------
@@ -298,6 +262,7 @@ bool SocketStreamTLS::WaitWhenRetryRequired(int SSLErrorCode, int Timeout)
 // --------------------------------------------------------------------------
 int SocketStreamTLS::Read(void *pBuffer, int NBytes, int Timeout)
 {
+	CheckForMissingTimeout(Timeout);
 	if(!mpSSL) {THROW_EXCEPTION(ServerException, TLSNoSSLObject)}
 
 	// Make sure zero byte reads work as expected
@@ -352,7 +317,7 @@ int SocketStreamTLS::Read(void *pBuffer, int NBytes, int Timeout)
 //		Created: 2003/08/06
 //
 // --------------------------------------------------------------------------
-void SocketStreamTLS::Write(const void *pBuffer, int NBytes)
+void SocketStreamTLS::Write(const void *pBuffer, int NBytes, int Timeout)
 {
 	if(!mpSSL) {THROW_EXCEPTION(ServerException, TLSNoSSLObject)}
 	
diff --git a/lib/server/SocketStreamTLS.h b/lib/server/SocketStreamTLS.h
index bb40ed10..3fda98c1 100644
--- a/lib/server/SocketStreamTLS.h
+++ b/lib/server/SocketStreamTLS.h
@@ -43,7 +43,8 @@ public:
 	void Handshake(const TLSContext &rContext, bool IsServer = false);
 	
 	virtual int Read(void *pBuffer, int NBytes, int Timeout = IOStream::TimeOutInfinite);
-	virtual void Write(const void *pBuffer, int NBytes);
+	virtual void Write(const void *pBuffer, int NBytes,
+		int Timeout = IOStream::TimeOutInfinite);
 	virtual void Close();
 	virtual void Shutdown(bool Read = true, bool Write = true);