Fix incorrect timeout calculation in GetLine

Attaching a debugger to a process waiting in GetLine would previously cause the
read() to return immediately with EINTR, which we never retried. Now it retries
until the original timeout has expired.

(cherry picked from commit e997d09)

lib/common/GetLine.cpp

@@ -15,8 +15,9 @@
 	#include <unistd.h>
 #endif
 
+#include "autogen_CommonException.h"
+#include "BoxTime.h"
 #include "GetLine.h"
-#include "CommonException.h"
 
 #include "MemLeakFindOn.h"
 
@@ -56,8 +57,8 @@ GetLine::GetLine()
 //		Created: 2011/04/22
 //
 // --------------------------------------------------------------------------
-bool GetLine::GetLineInternal(std::string &rOutput, bool Preprocess,
-	int Timeout)
+bool GetLine::GetLineInternal(std::string &rOutput, bool preprocess,
+	int timeout)
 {
 	// EOF?
 	if(mEOF) {THROW_EXCEPTION(CommonException, GetLineEOF)}
@@ -66,9 +67,12 @@ bool GetLine::GetLineInternal(std::string &rOutput, bool Preprocess,
 	rOutput = mPendingString;
 	mPendingString.erase();
 
-	bool foundLineEnd = false;
+	box_time_t start_time = GetCurrentBoxTime();
+	box_time_t remaining_time = MilliSecondsToBoxTime(timeout);
+	box_time_t end_time = start_time + remaining_time;
+	bool found_line_end = false;
 
-	while(!foundLineEnd && !mEOF)
+	while(!found_line_end && !mEOF)
 	{
 		// Use any bytes left in the buffer
 		while(mBufferBegin < mBytesInBuffer)
@@ -81,7 +85,7 @@ bool GetLine::GetLineInternal(std::string &rOutput, bool Preprocess,
 			else if(c == '\n')
 			{
 				// Line end!
-				foundLineEnd = true;
+				found_line_end = true;
 				break;
 			}
 			else
@@ -93,7 +97,7 @@ bool GetLine::GetLineInternal(std::string &rOutput, bool Preprocess,
 			// Implicit line ending at EOF
 			if(mBufferBegin >= mBytesInBuffer && mPendingEOF)
 			{
-				foundLineEnd = true;
+				found_line_end = true;
 			}
 		}
 
@@ -102,11 +106,36 @@ bool GetLine::GetLineInternal(std::string &rOutput, bool Preprocess,
 		{
 			THROW_EXCEPTION(CommonException, GetLineTooLarge)
 		}
-
+
+		if(timeout != IOStream::TimeOutInfinite)
+		{
+			// Update remaining time, and if we have run out and not yet found EOL, then
+			// stash what we've read so far, and return false. (If the timeout is infinite,
+			// the only way out is EOL or EOF.)
+			remaining_time = end_time - GetCurrentBoxTime();
+			if(!found_line_end && remaining_time < 0)
+			{
+			       mPendingString = rOutput;
+			       return false;
+			}
+		}
+
 		// Read more in?
-		if(!foundLineEnd && mBufferBegin >= mBytesInBuffer && !mPendingEOF)
+		if(!found_line_end && mBufferBegin >= mBytesInBuffer && !mPendingEOF)
 		{
-			int bytes = ReadMore(Timeout);
+			int64_t read_timeout_ms;
+			if(timeout == IOStream::TimeOutInfinite)
+			{
+				read_timeout_ms = IOStream::TimeOutInfinite;
+			}
+			else
+			{
+				// We should have exited above, if remaining_time < 0.
+				ASSERT(remaining_time >= 0);
+				read_timeout_ms = BoxTimeToMilliSeconds(remaining_time);
+			}
+
+			int bytes = ReadMore(read_timeout_ms);
 
 			// Error?
 			if(bytes == -1)
@@ -117,7 +146,7 @@ bool GetLine::GetLineInternal(std::string &rOutput, bool Preprocess,
 			// Adjust buffer info
 			mBytesInBuffer = bytes;
 			mBufferBegin = 0;
-			
+
 			// No data returned?
 			if(bytes == 0 && IsStreamDataLeft())
 			{
@@ -136,7 +165,7 @@ bool GetLine::GetLineInternal(std::string &rOutput, bool Preprocess,
 		}
 	}
 
-	if(Preprocess)
+	if(preprocess)
 	{
 		// Check for comment char, but char before must be whitespace
 		// end points to a gap between characters, may equal start if

test/basicserver/testbasicserver.cpp

@@ -33,8 +33,8 @@
 #define SERVER_LISTEN_PORT	2003
 
 // in ms
-#define COMMS_READ_TIMEOUT					4
-#define COMMS_SERVER_WAIT_BEFORE_REPLYING	40
+#define COMMS_READ_TIMEOUT	4
+#define COMMS_SERVER_WAIT_BEFORE_REPLYING	1000
 // Use a longer timeout to give Srv2TestConversations time to write 20 MB to each of
 // three child processes before starting to read it back again, without the children
 // timing out and aborting.
@@ -67,9 +67,10 @@ void testservers_pause_before_reply()
 #ifdef WIN32
 	Sleep(COMMS_SERVER_WAIT_BEFORE_REPLYING);
 #else
+	int64_t nsec = COMMS_SERVER_WAIT_BEFORE_REPLYING * 1000LL * 1000;	// convert to ns
 	struct timespec t;
-	t.tv_sec = 0;
-	t.tv_nsec = COMMS_SERVER_WAIT_BEFORE_REPLYING * 1000 * 1000;	// convert to ns
+	t.tv_sec = nsec / NANO_SEC_IN_SEC;
+	t.tv_nsec = nsec % NANO_SEC_IN_SEC;
 	::nanosleep(&t, NULL);
 #endif
 }
@@ -91,7 +92,11 @@ void testservers_connection(SocketStream &rStream)
 		std::string line1("CONNECTED:");
 		line1 += rtls.GetPeerCommonName();
 		line1 += '\n';
+
+		// Reply after a short delay, to allow the client to test timing out
+		// in GetLine():
 		testservers_pause_before_reply();
+
 		rStream.Write(line1.c_str(), line1.size());
 	}
 
@@ -313,7 +318,6 @@ void Srv2TestConversations(const std::vector<IOStream *> &conns)
 	{
 		getline[c] = new IOStreamGetLine(*conns[c]);
 
-		bool hadTimeout = false;
 		if(typeid(*conns[c]) == typeid(SocketStreamTLS))
 		{
 			SocketStreamTLS *ptls = (SocketStreamTLS *)conns[c];
@@ -323,10 +327,18 @@ void Srv2TestConversations(const std::vector<IOStream *> &conns)
 			conns[c]->Write("Hello\n", 6);
 
 			std::string line1;
-			while(!getline[c]->GetLine(line1, false, COMMS_READ_TIMEOUT))
-				hadTimeout = true;
-			TEST_THAT(line1 == "CONNECTED:CLIENT");
-			TEST_THAT(hadTimeout)
+
+			// First read should timeout, while server sleeps in
+			// testservers_pause_before_reply():
+			TEST_THAT(!getline[c]->GetLine(line1, false,
+				COMMS_SERVER_WAIT_BEFORE_REPLYING * 0.5));
+			TEST_EQUAL(line1, "");
+
+			// Second read should not timeout, because we should have waited
+			// COMMS_SERVER_WAIT_BEFORE_REPLYING * 1.5
+			TEST_THAT(getline[c]->GetLine(line1, false,
+				COMMS_SERVER_WAIT_BEFORE_REPLYING));
+			TEST_EQUAL(line1, "CONNECTED:CLIENT");
 		}
 	}