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

diff --git a/docs/changes.txt b/docs/changes.txt
index 2a3801d200cc586713d3f33c94061cf19fd1c709..6e4dd762f1e65d2cd333f07e97b57b9d9abfec39 100644 (file)
--- 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 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):
 
 
 All (GUI):
 

diff --git a/src/unix/evtloopunix.cpp b/src/unix/evtloopunix.cpp
index b219d1b2dca30138516d60b68f9df0b3329b38ae..98b612cae5108d9951029f29a965bbd897373f07 100644 (file)
--- 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
 public:
     // default ctor does nothing, call Create() to really initialize the
     // object

-    PipeIOHandler() { }
+    PipeIOHandler() : m_pipeIsEmpty(false) { }

 
     bool Create();
 
 
     bool Create();
 


@@ -74,6 +74,15 @@ public:
 
 private:
     wxPipe m_pipe;
 
 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;
     }
 
         return false;
     }
 

+

     if ( !m_pipe.MakeNonBlocking(wxPipe::Read) )
     {
         wxLogSysError(_("Failed to switch wake up pipe to non-blocking mode"));
     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()
 {
 
 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)");
     }
     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()
 {
 }
 
 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));
 
     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;
         }
 
             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;
         }
             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
     // 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