Explorar el Código

BUG: Fixed heap corruption error by log handlers

Problem:
When a multi-threaded filter logged a message during execution it
often happened that multiple threads logged a message at the same time,
using the same message handler. The message handler contained a single
QRegExp ContextRegExp object, which was used from multiple threads
at the same time and caused memory corruption and crash.

Analysis:
Potential solutions could be: lock the QRegExp object so that only one
thread can use it at a time or create a new QRegExp for each thread.
Long-term nice solution would be to change ITK and VTK to provide
interface for structured logging (send filename, line number, log level,
message separately) instead of dumping a text message that we have to
parse.

Solution:
Creating a new QRegExp for each thread is simpler and probably
not slower, therefore this option is implemented.

Note:
While all methods of QRegExp are documented to be reentrant, there is
probably an issue in the implementation. As it was already reported in
QTBUG-20450 [1]

[1] https://bugreports.qt.io/browse/QTBUG-20450

Fixes #546

Reviewed-by: Jean-Christophe Fillion-Robin <jchris.fillionr@kitware.com>
Andras Lasso hace 10 años
padre
commit
465d198b90

+ 9 - 8
Libs/ImageProcessing/ITK/Core/ctkITKErrorLogMessageHandler.cpp

@@ -59,8 +59,8 @@ public:
   /** Run-time type information (and related methods). */
   itkTypeMacro(ctkITKOutputWindow, OutputWindow);
 
-  ctkITKOutputWindow():MessageHandler(0),
-    ContextRegExp("[a-zA-Z\\s]+: In (.+), line ([\\d]+)\\n(.+\\(0x[a-fA-F0-9]+\\))\\:\\s(.*)"){}
+  ctkITKOutputWindow():MessageHandler(0)
+  {}
   ~ctkITKOutputWindow(){}
 
   virtual void DisplayText(const char*);
@@ -74,7 +74,6 @@ public:
 
   ctkErrorLogAbstractMessageHandler * MessageHandler;
 
-  QRegExp ContextRegExp;
 };
 
 // --------------------------------------------------------------------------
@@ -142,12 +141,14 @@ void ctkITKOutputWindow::DisplayDebugText(const char* text)
 QString ctkITKOutputWindow::parseText(const QString& text, ctkErrorLogContext& context)
 {
   context.Message = text;
-  if (this->ContextRegExp.exactMatch(text))
+
+  QRegExp contextRegExp("[a-zA-Z\\s]+: In (.+), line ([\\d]+)\\n(.+\\(0x[a-fA-F0-9]+\\))\\:\\s(.*)");
+  if (contextRegExp.exactMatch(text))
     {
-    context.File = this->ContextRegExp.cap(1);
-    context.Category = this->ContextRegExp.cap(3);
-    context.Line = this->ContextRegExp.cap(2).toInt();
-    context.Message = this->ContextRegExp.cap(4);
+    context.File = contextRegExp.cap(1);
+    context.Category = contextRegExp.cap(3);
+    context.Line = contextRegExp.cap(2).toInt();
+    context.Message = contextRegExp.cap(4);
     }
   return context.Message;
 }

+ 6 - 8
Libs/Visualization/VTK/Core/ctkVTKErrorLogMessageHandler.cpp

@@ -45,7 +45,6 @@ public:
 
   ctkVTKOutputWindow()
     : MessageHandler(0)
-    , ContextRegExp("[a-zA-Z\\s]+: In (.+), line ([\\d]+)\\n(.+\\((?:0x)?[a-fA-F0-9]+\\))\\:\\s(.*)")
   {}
   ~ctkVTKOutputWindow(){}
 
@@ -59,8 +58,6 @@ public:
   QString parseText(const QString &text, ctkErrorLogContext &context);
 
   ctkErrorLogAbstractMessageHandler * MessageHandler;
-
-  QRegExp ContextRegExp;
 };
 
 // --------------------------------------------------------------------------
@@ -141,12 +138,13 @@ void ctkVTKOutputWindow::DisplayDebugText(const char* text)
 QString ctkVTKOutputWindow::parseText(const QString& text, ctkErrorLogContext& context)
 {
   context.Message = text;
-  if (this->ContextRegExp.exactMatch(text))
+  QRegExp contextRegExp("[a-zA-Z\\s]+: In (.+), line ([\\d]+)\\n(.+\\((?:0x)?[a-fA-F0-9]+\\))\\:\\s(.*)");
+  if (contextRegExp.exactMatch(text))
     {
-    context.File = this->ContextRegExp.cap(1);
-    context.Category = this->ContextRegExp.cap(3);
-    context.Line = this->ContextRegExp.cap(2).toInt();
-    context.Message = this->ContextRegExp.cap(4);
+    context.File = contextRegExp.cap(1);
+    context.Category = contextRegExp.cap(3);
+    context.Line = contextRegExp.cap(2).toInt();
+    context.Message = contextRegExp.cap(4);
     }
   return context.Message;
 }