From 589046c0bd6abc0836ffc97a6f4ebceea35a79c2 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Wed, 4 Apr 2012 14:36:45 +0000 Subject: [PATCH] Avoid overflowing the wake up when handling events in Unix console apps. Generating many wake ups from the worker threads could result in overflowing the buffer of the pipe used to communicate with the main thread which, in turn, resulted in other serious problems (deadlocks...). Avoid this by only writing to the pipe if it is empty. Closes #14166. git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@71089 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775 --- docs/changes.txt | 1 + src/unix/evtloopunix.cpp | 57 +++++++++++++++++++++++++++++++++------- 2 files changed, 48 insertions(+), 10 deletions(-) diff --git a/docs/changes.txt b/docs/changes.txt index 2a3801d200..6e4dd762f1 100644 --- a/docs/changes.txt +++ b/docs/changes.txt @@ -474,6 +474,7 @@ All: - Fix performance of wxStdInputStream with MSVC8/9 (wsu). - Added wxFileName::Exists(). - Implement wxThread::SetConcurrency() for POSIX systems (Igor Korot). +- Fix deadlock due to too many events in Unix console apps (Lukasz Michalski). All (GUI): diff --git a/src/unix/evtloopunix.cpp b/src/unix/evtloopunix.cpp index b219d1b2dc..98b612cae5 100644 --- a/src/unix/evtloopunix.cpp +++ b/src/unix/evtloopunix.cpp @@ -58,7 +58,7 @@ class PipeIOHandler : public wxFDIOHandler public: // default ctor does nothing, call Create() to really initialize the // object - PipeIOHandler() { } + PipeIOHandler() : m_pipeIsEmpty(false) { } bool Create(); @@ -74,6 +74,15 @@ public: private: wxPipe m_pipe; + + // Protects access to m_pipeIsEmpty. + wxCriticalSection m_pipeLock; + + // This flag is set to true after writing to the pipe and reset to false + // after reading from it in the main thread. Having it allows us to avoid + // overflowing the pipe with too many writes if the main thread can't keep + // up with reading from it. + bool m_pipeIsEmpty; }; // ---------------------------------------------------------------------------- @@ -88,6 +97,7 @@ bool PipeIOHandler::Create() return false; } + if ( !m_pipe.MakeNonBlocking(wxPipe::Read) ) { wxLogSysError(_("Failed to switch wake up pipe to non-blocking mode")); @@ -106,39 +116,66 @@ bool PipeIOHandler::Create() void PipeIOHandler::WakeUp() { + wxCriticalSectionLocker lock(m_pipeLock); + + // No need to do anything if the pipe already contains something. + if ( !m_pipeIsEmpty ) + return; + if ( write(m_pipe[wxPipe::Write], "s", 1) != 1 ) { // don't use wxLog here, we can be in another thread and this could // result in dead locks perror("write(wake up pipe)"); } + else + { + // We just wrote to it, so it's not empty any more. + m_pipeIsEmpty = false; + } } void PipeIOHandler::OnReadWaiting() { - // got wakeup from child thread: read all data available in pipe just to - // make it empty (even though we write one byte at a time from WakeUp(), - // it could have been called several times) + // got wakeup from child thread, remove the data that provoked it from the + // pipe + + wxCriticalSectionLocker lock(m_pipeLock); + char buf[4]; for ( ;; ) { const int size = read(GetReadFd(), buf, WXSIZEOF(buf)); - if ( size == 0 || (size == -1 && (errno == EAGAIN || errno == EINTR)) ) + if ( size > 0 ) { - // nothing left in the pipe (EAGAIN is expected for an FD with - // O_NONBLOCK) + wxASSERT_MSG( size == 1, "Too many writes to wake-up pipe?" ); + break; } - if ( size == -1 ) + if ( size == 0 || (size == -1 && errno == EAGAIN) ) { - wxLogSysError(_("Failed to read from wake-up pipe")); - + // No data available, not an error (but still surprising, + // spurious wakeup?) break; } + + if ( errno == EINTR ) + { + // We were interrupted, try again. + continue; + } + + wxLogSysError(_("Failed to read from wake-up pipe")); + + return; } + // The pipe is empty now, so future calls to WakeUp() would need to write + // to it again. + m_pipeIsEmpty = true; + // writing to the wake up pipe will make wxConsoleEventLoop return from // wxFDIODispatcher::Dispatch() it might be currently blocking in, nothing // else needs to be done -- 2.45.2