Browse Source

ctkFDHandler - The polling thread now terminates graciously

Before that change the thread in charge of reading the content
of either stderr or stdout descriptor was terminated using:

  this->terminate()
  this->wait();

On some operating system, using this strategy was leaving the thread in
a inconsistent state. See http://na-mic.org/Mantis/view.php?id=1537

The approach implemented here is less brutal. Indeed, the "Enabled" boolean
variable is protected by a QMutex and allows both the main thread and
the polling thread to either set or get the Enabled variable in a
thread-safe way.
Jean-Christophe Fillion-Robin 13 years ago
parent
commit
90f5d96828

+ 26 - 2
Libs/Core/ctkErrorLogFDMessageHandler.cpp

@@ -118,6 +118,7 @@ void ctkFDHandler::setEnabled(bool value)
 #endif
 
     // Start polling thread
+    this->Enabled = true;
     this->start();
     }
   else
@@ -129,7 +130,19 @@ void ctkFDHandler::setEnabled(bool value)
     this->RedirectionFile.flush();
 
     // Stop polling thread
-    this->terminate();
+    {
+      QMutexLocker locker(&this->EnableMutex);
+      this->Enabled = false;
+    }
+
+    QString newline("\n");
+#ifdef Q_OS_WIN32
+    _write(fileno(this->terminalOutputFile()), qPrintable(newline), newline.size());
+#else
+    write(fileno(this->terminalOutputFile()), qPrintable(newline), newline.size());
+#endif
+
+    // Wait the polling thread graciously terminates
     this->wait();
 
     // Close files and restore standard output to stdout or stderr - which should be the terminal
@@ -156,8 +169,13 @@ void ctkFDHandler::setEnabled(bool value)
     {
     terminalOutput->setFileDescriptor(this->SavedFDNumber);
     }
+}
 
-  this->Enabled = value;
+// --------------------------------------------------------------------------
+bool ctkFDHandler::enabled()const
+{
+  QMutexLocker locker(&this->EnableMutex);
+  return this->Enabled;
 }
 
 // --------------------------------------------------------------------------
@@ -183,6 +201,12 @@ void ctkFDHandler::run()
         line += c;
         }
       }
+
+    if (!this->enabled())
+      {
+      break;
+      }
+
     Q_ASSERT(this->MessageHandler);
     this->MessageHandler->handleMessage(
       ctk::qtHandleToString(QThread::currentThreadId()),

+ 8 - 0
Libs/Core/ctkErrorLogFDMessageHandler_p.h

@@ -23,6 +23,7 @@
 
 // Qt includes
 #include <QFile>
+#include <QMutex>
 #include <QThread>
 
 // CTK includes
@@ -42,6 +43,7 @@ class QTextStream;
 class ctkFDHandler : public QThread
 {
   Q_OBJECT
+  Q_PROPERTY(bool enabled READ enabled WRITE setEnabled)
 public:
   typedef ctkFDHandler Self;
 
@@ -50,8 +52,12 @@ public:
                ctkErrorLogModel::TerminalOutput terminalOutput);
   virtual ~ctkFDHandler();
 
+  /// Enable/Disable the handler.
   void setEnabled(bool value);
 
+  /// Return if the handler is enabled. This methods is thread-safe.
+  bool enabled()const;
+
   FILE* terminalOutputFile();
 
 protected:
@@ -73,6 +79,8 @@ private:
   QTextStream* RedirectionStream;
 
   bool Initialized;
+
+  mutable QMutex EnableMutex;
   bool Enabled;
 };