attempting to solve ipc recursion/deadlock problem by always buffering in the log outputter.

This commit is contained in:
Nick Bolton 2012-07-08 16:01:27 +00:00
parent af9a6beb78
commit f0493351a1
3 changed files with 69 additions and 55 deletions

View File

@ -24,26 +24,23 @@
#include "TMethodEventJob.h"
#include "CIpcClientProxy.h"
#include "CArch.h"
#include "CThread.h"
#include "TMethodJob.h"
CIpcLogOutputter::CIpcLogOutputter(CIpcServer& ipcServer) :
m_ipcServer(ipcServer),
m_sending(false)
m_bufferMutex(ARCH->newMutex()),
m_sending(false),
m_running(true)
{
m_mutex = ARCH->newMutex();
m_bufferThread = new CThread(new TMethodJob<CIpcLogOutputter>(
this, &CIpcLogOutputter::bufferThread));
}
CIpcLogOutputter::~CIpcLogOutputter()
{
}
void
CIpcLogOutputter::sendBuffer(CIpcClientProxy& proxy)
{
CIpcMessage message;
message.m_type = kIpcLogLine;
message.m_data = new CString(m_buffer);
proxy.send(message);
m_buffer.clear();
ARCH->closeMutex(m_bufferMutex);
delete m_bufferThread;
}
void
@ -54,6 +51,8 @@ CIpcLogOutputter::open(const char* title)
void
CIpcLogOutputter::close()
{
m_running = false;
m_bufferThread->wait(5);
}
void
@ -62,37 +61,59 @@ CIpcLogOutputter::show(bool showIfEmpty)
}
bool
CIpcLogOutputter::write(ELevel level, const char* text)
CIpcLogOutputter::write(ELevel, const char* text)
{
// drop messages logged while sending over ipc, since ipc can cause
// log messages (sending these could cause recursion or deadlocks).
// this has the side effect of dropping messages from other threads
// which weren't caused by ipc, but that is just the downside of
// logging this way.
// sending the buffer generates log messages, which we must throw
// away (otherwise this would cause recursion). this is just a drawback
// of logging this way. there is also the risk that this could throw
// away log messages not generated by the ipc, but it seems like it
// would be difficult to distinguish (other than looking at the stack
// trace somehow). perhaps a file stream might be a better option :-/
if (m_sending) {
return false;
}
// protect the value of m_sending.
CArchMutexLock lock(m_mutex);
m_sending = true;
try {
if (m_ipcServer.hasClients(kIpcClientGui)) {
CIpcMessage message;
message.m_type = kIpcLogLine;
message.m_data = new CString(text);
m_ipcServer.send(message, kIpcClientGui);
}
else {
m_buffer.append(text);
m_buffer.append("\n");
}
m_sending = false;
return true;
}
catch (...) {
m_sending = false;
throw;
CArchMutexLock lock(m_bufferMutex);
m_buffer.append(text);
m_buffer.append("\n");
return true;
}
void
CIpcLogOutputter::bufferThread(void*)
{
while (m_running) {
while (m_running && m_buffer.size() == 0) {
ARCH->sleep(.1);
}
if (!m_running) {
break;
}
if (m_ipcServer.hasClients(kIpcClientGui)) {
sendBuffer();
}
}
}
CString*
CIpcLogOutputter::emptyBuffer()
{
CArchMutexLock lock(m_bufferMutex);
CString* copy = new CString(m_buffer);
m_buffer.clear();
return copy;
}
void
CIpcLogOutputter::sendBuffer()
{
CIpcMessage message;
message.m_type = kIpcLogLine;
message.m_data = emptyBuffer();
m_sending = true;
m_ipcServer.send(message, kIpcClientGui);
m_sending = false;
}

View File

@ -18,6 +18,7 @@
#pragma once
#include "ILogOutputter.h"
#include "CArch.h"
class CIpcServer;
class CEvent;
@ -38,12 +39,16 @@ public:
virtual void show(bool showIfEmpty);
virtual bool write(ELevel level, const char* message);
//! Sends messages queued while no clients were connected.
void sendBuffer(CIpcClientProxy& proxy);
private:
void bufferThread(void*);
CString* emptyBuffer();
void sendBuffer();
private:
CIpcServer& m_ipcServer;
CString m_buffer;
CArchMutex m_mutex;
CArchMutex m_bufferMutex;
bool m_sending;
CThread* m_bufferThread;
bool m_running;
};

View File

@ -330,17 +330,5 @@ CDaemonApp::handleIpcMessage(const CEvent& e, void*)
m_relauncher->command(command);
break;
}
case kIpcHello: {
if (m.m_source != nullptr) {
CIpcClientProxy& proxy = *static_cast<CIpcClientProxy*>(m.m_source);
if (proxy.m_clientType == kIpcClientGui) {
// when a new gui client connects, send them all the
// log messages queued up while they were gone.
m_ipcLogOutputter->sendBuffer(proxy);
}
}
break;
}
}
}