Solution attempt for timing bugs in write_bufferRateLimit

It's probably better to wait until the buffer is sent, rather than
waiting until its empty. To test the output it has to be sent, but
because of timing, it may be emptied at any point.

src/lib/ipc/IpcLogOutputter.cpp

@@ -46,8 +46,6 @@ IpcLogOutputter::IpcLogOutputter(IpcServer& ipcServer) :
 	m_notifyMutex(ARCH->newMutex()),
 	m_bufferWaiting(false),
 	m_bufferMaxSize(kBufferMaxSize),
-	m_bufferEmptyCond(ARCH->newCondVar()),
-	m_bufferEmptyMutex(ARCH->newMutex()),
 	m_bufferRateWriteLimit(kBufferRateWriteLimit),
 	m_bufferRateTimeLimit(kBufferRateTimeLimit),
 	m_bufferWriteCount(0),
@@ -66,13 +64,6 @@ IpcLogOutputter::~IpcLogOutputter()
 
 	ARCH->closeCondVar(m_notifyCond);
 	ARCH->closeMutex(m_notifyMutex);
-
-	ARCH->closeCondVar(m_bufferEmptyCond);
-	
-#ifndef WINAPI_CARBON
-	// HACK: assert fails on mac debug, can't see why.
-	ARCH->closeMutex(m_bufferEmptyMutex);
-#endif // WINAPI_CARBON
 }
 
 void
@@ -172,11 +163,6 @@ IpcLogOutputter::bufferThread(void*)
 				break;
 			}
 
-			if (m_buffer.empty()) {
-				ArchMutexLock lock(m_bufferEmptyMutex);
-				ARCH->broadcastCondVar(m_bufferEmptyCond);
-			}
-
 			m_bufferWaiting = true;
 			ARCH->waitCondVar(m_notifyCond, m_notifyMutex, -1);
 			m_bufferWaiting = false;
@@ -239,12 +225,6 @@ IpcLogOutputter::bufferMaxSize() const
 	return m_bufferMaxSize;
 }
 
-void
-IpcLogOutputter::waitForEmpty()
-{
-	ARCH->waitCondVar(m_bufferEmptyCond, m_bufferEmptyMutex, -1);
-}
-
 void
 IpcLogOutputter::bufferRateLimit(UInt16 writeLimit, double timeLimit)
 {

src/lib/ipc/IpcLogOutputter.h

@@ -106,8 +106,6 @@ private:
 	IArchMultithread::ThreadID
 						m_bufferThreadId;
 	UInt16				m_bufferMaxSize;
-	ArchCond			m_bufferEmptyCond;
-	ArchMutex			m_bufferEmptyMutex;
 	UInt16				m_bufferRateWriteLimit;
 	double				m_bufferRateTimeLimit;
 	UInt16				m_bufferWriteCount;

src/test/mock/ipc/MockIpcServer.h

@@ -19,17 +19,40 @@
 
 #include "ipc/IpcServer.h"
 #include "ipc/IpcMessage.h"
+#include "arch/Arch.h"
 
 #include "test/global/gmock.h"
 
+using ::testing::_;
+using ::testing::Invoke;
+
 class IEventQueue;
 
 class MockIpcServer : public IpcServer
 {
 public:
-	MockIpcServer() { }
+	MockIpcServer() :
+		m_sendCond(ARCH->newCondVar()),
+		m_sendMutex(ARCH->newMutex()) { }
 
 	MOCK_METHOD0(listen, void());
 	MOCK_METHOD2(send, void(const IpcMessage&, EIpcClientType));
 	MOCK_CONST_METHOD1(hasClients, bool(EIpcClientType));
+
+	void delegateToFake() {
+		ON_CALL(*this, send(_, _)).WillByDefault(Invoke(this, &MockIpcServer::mockSend));
+	}
+
+	void waitForSend() {
+		ARCH->waitCondVar(m_sendCond, m_sendMutex, -1);
+	}
+
+private:
+	void mockSend(const IpcMessage&, EIpcClientType) {
+		ArchMutexLock lock(m_sendMutex);
+		ARCH->broadcastCondVar(m_sendCond);
+	}
+
+	ArchCond			m_sendCond;
+	ArchMutex			m_sendMutex;
 };

src/test/unittests/ipc/IpcLogOutputterTests.cpp

@@ -48,6 +48,7 @@ inline const Matcher<const IpcMessage&> IpcLogLineMessageEq(const String& s) {
 TEST(IpcLogOutputterTests, write_bufferSizeWrapping)
 {
 	MockIpcServer mockServer;
+	mockServer.delegateToFake();
 	
 	ON_CALL(mockServer, hasClients(_)).WillByDefault(Return(true));
 
@@ -61,14 +62,13 @@ TEST(IpcLogOutputterTests, write_bufferSizeWrapping)
 	outputter.write(kNOTE, "mock 1");
 	outputter.write(kNOTE, "mock 2");
 	outputter.write(kNOTE, "mock 3");
-
-	// wait for the buffer to be empty (all lines sent to IPC)
-	outputter.waitForEmpty();
+	mockServer.waitForSend();
 }
 
 TEST(IpcLogOutputterTests, write_bufferRateLimit)
 {
 	MockIpcServer mockServer;
+	mockServer.delegateToFake();
 	
 	ON_CALL(mockServer, hasClients(_)).WillByDefault(Return(true));
 
@@ -82,14 +82,13 @@ TEST(IpcLogOutputterTests, write_bufferRateLimit)
 	// log 1 more line than the buffer can accept in time limit.
 	outputter.write(kNOTE, "mock 1");
 	outputter.write(kNOTE, "mock 2");
-	outputter.waitForEmpty();
+	mockServer.waitForSend();
 	
 	// after waiting the time limit send another to make sure
 	// we can log after the time limit passes.
-	ARCH->sleep(0.01); // 10ms
 	outputter.write(kNOTE, "mock 3");
 	outputter.write(kNOTE, "mock 4");
-	outputter.waitForEmpty();
+	mockServer.waitForSend();
 }
 
 #endif // WINAPI_MSWINDOWS