Fixed|liblegacy|libappfw: Improved thread-safety

Threads should not be modified after they’ve already been started.
skyjake · Feb 10, 2017 · 813e97e · 813e97e
1 parent cf3d754
commit 813e97e
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 25 deletions.
diff --git a/doomsday/sdk/libappfw/src/fontlinewrapping.cpp b/doomsday/sdk/libappfw/src/fontlinewrapping.cpp
@@ -25,6 +25,7 @@
 #include <de/Image>
 
 #include <QMap>
+#include <atomic>
 
 namespace de {
 
@@ -75,7 +76,7 @@ DENG2_PIMPL_NOREF(FontLineWrapping)
     int indent;                 ///< Current left indentation (in pixels).
     QVector<int> prevIndents;
     int tabStop;
-    volatile bool cancelled = false;
+    std::atomic_bool cancelled { false };
 
     DENG2_ERROR(CancelError);
 
@@ -762,12 +763,15 @@ Vector2i FontLineWrapping::charTopLeftInPixels(int line, int charIndex)
 
 FontLineWrapping::LineInfo const &FontLineWrapping::lineInfo(int index) const
 {
+    DENG2_GUARD(this);
     DENG2_ASSERT(index >= 0 && index < d->lines.size());
     return d->lines[index]->info;
 }
 
 void FontLineWrapping::rasterizeLines(Rangei const &lineRange)
 {
+    DENG2_GUARD(this);
+
     d->rasterized.clear();
 
     for (int i = 0; i < height(); ++i)
@@ -787,19 +791,24 @@ void FontLineWrapping::rasterizeLines(Rangei const &lineRange)
 
 void FontLineWrapping::clearRasterizedLines() const
 {
+    DENG2_GUARD(this);
     d->rasterized.clear();
 }
 
 Image FontLineWrapping::rasterizedSegment(int line, int segment) const
 {
-    DENG2_ASSERT(line >= 0);
-    if (line >= 0 && line < d->rasterized.size())
+    // Check the cached images.
     {
-        auto const &rasterLine = d->rasterized.at(line);
-        if (!rasterLine.segmentImages.isEmpty())
+        DENG2_GUARD(this);
+        DENG2_ASSERT(line >= 0);
+        if (line >= 0 && line < d->rasterized.size())
         {
-            DENG2_ASSERT(segment >= 0 && segment < rasterLine.segmentImages.size());
-            return rasterLine.segmentImages.at(segment);
+            auto const &rasterLine = d->rasterized.at(line);
+            if (!rasterLine.segmentImages.isEmpty())
+            {
+                DENG2_ASSERT(segment >= 0 && segment < rasterLine.segmentImages.size());
+                return rasterLine.segmentImages.at(segment);
+            }
         }
     }
     // Rasterize now, since it wasn't previously rasterized.

diff --git a/doomsday/sdk/liblegacy/include/de/concurrency.h b/doomsday/sdk/liblegacy/include/de/concurrency.h
@@ -76,7 +76,19 @@ protected slots:
 
 #endif // __DENG__
 
-DENG_PUBLIC thread_t Sys_StartThread(systhreadfunc_t startpos, void *parm);
+/**
+ * Starts a new thread.
+ *
+ * @param startpos  Executes while the thread is running. When the function exists,
+ *                  the thread stops.
+ * @param parm             Parameter given to the thread function.
+ * @param terminationFunc  Callback function that is called from the worker thread 
+ *                         right before it exits. The callback is given the exit status
+ *                         of the thread as a parameter.
+ * @return Thread handle.
+ */
+DENG_PUBLIC thread_t Sys_StartThread(systhreadfunc_t startpos, void *parm,
+                                     void (*terminationFunc)(systhreadexitstatus_t));
 
 extern "C" {
 #endif // __cplusplus
@@ -92,28 +104,21 @@ extern "C" {
 #endif
 
 /**
- * Stars a new thread with the given callback.
+ * Starts a new thread.
  *
  * @param startpos  Executes while the thread is running. When the function exists,
  *                  the thread stops.
- * @param parm      Parameter passed to the callback.
- *
+ * @param parm             Parameter given to the thread function.
+ * @param terminationFunc  Callback function that is called from the worker thread
+ *                         right before it exits. The callback is given the exit status
+ *                         of the thread as a parameter.
  * @return Thread handle.
  */
-DENG_PUBLIC thread_t Sys_StartThread(int (*startpos)(void *), void *parm);
+DENG_PUBLIC thread_t Sys_StartThread(int (*startpos)(void *), void *parm,
+                                     void (*terminationFunc)(systhreadexitstatus_t));
 
 DENG_PUBLIC void Thread_Sleep(int milliseconds);
 
-/**
- * Sets a callback function that is called from the worker thread right before
- * it exits. The callback is given the exit status of the thread as a
- * parameter.
- *
- * @param thread           Thread handle.
- * @param terminationFunc  Callback to call before terminating the thread.
- */
-DENG_PUBLIC void Thread_SetCallback(thread_t thread, void (*terminationFunc)(systhreadexitstatus_t));
-
 /**
  * Wait for a thread to stop. If the thread does not stop after @a timeoutMs,
  * it will be forcibly terminated.

diff --git a/doomsday/sdk/liblegacy/src/concurrency.cpp b/doomsday/sdk/liblegacy/src/concurrency.cpp
@@ -127,16 +127,17 @@ void Thread_Sleep(int milliseconds)
     de::TimeDelta::fromMilliSeconds(milliseconds).sleep();
 }
 
-thread_t Sys_StartThread(systhreadfunc_t startpos, void *parm)
+thread_t Sys_StartThread(systhreadfunc_t startpos, void *parm, void (*terminationFunc)(systhreadexitstatus_t))
 {
     CallbackThread *t = new CallbackThread(startpos, parm);
+    t->setTerminationFunc(terminationFunc);
     t->start();
     return t;
 }
 
-thread_t Sys_StartThread(int (*startpos)(void *), void *parm)
+thread_t Sys_StartThread(int (*startpos)(void *), void *parm, void (*terminationFunc)(systhreadexitstatus_t))
 {
-    return Sys_StartThread(systhreadfunc_t(startpos), parm);
+    return Sys_StartThread(systhreadfunc_t(startpos), parm, terminationFunc);
 }
 
 void Thread_KillAbnormally(thread_t handle)