libcore: Improving thread-safety

doomsday/sdk/libcore/include/de/concurrency/lockable.h

@@ -26,6 +26,8 @@
 #include <QMutex>
 
 namespace de {
+
+//#define DENG2_LOCKABLE_LOCK_COUNT
 
 /**
  * A mutex that can be used to synchronize access to a resource.  All classes
@@ -47,15 +49,15 @@ class DENG2_PUBLIC Lockable
     /// Release the lock.
     void unlock() const;
 
-#ifdef DENG2_DEBUG
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
     /// Returns true, if the lock is currently locked.
     bool isLocked() const;
 #endif
 
-private:
+public:
     mutable QMutex _mutex;
 
-#ifdef DENG2_DEBUG
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
     mutable int _lockCount;
     mutable QMutex _countMutex;
 #endif

doomsday/sdk/libcore/include/de/core/id.h

@@ -100,8 +100,6 @@ class DENG2_PUBLIC Id : public ISerializable, public LogEntry::Arg::Base
 
 private:
     Type _id;
-
-    static Type _generator;
 };
 
 DENG2_PUBLIC QTextStream &operator << (QTextStream &os, Id const &id);

doomsday/sdk/libcore/src/concurrency/lockable.cpp

@@ -25,21 +25,23 @@ namespace de {
 
 Lockable::Lockable()
     : _mutex(QMutex::Recursive)
-#ifdef DENG2_DEBUG
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
     , _lockCount(0)
 #endif
 {}
 
 Lockable::~Lockable()
 {
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
     DENG2_ASSERT(!isLocked()); // You should unlock before deleting!
+#endif
 }
 
 void Lockable::lock() const
 {
     _mutex.lock();
 
-#ifdef DENG2_DEBUG
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
     _countMutex.lock();
     _lockCount++;
     _countMutex.unlock();
@@ -48,21 +50,21 @@ void Lockable::lock() const
 
 void Lockable::unlock() const
 {
-#ifdef DENG2_DEBUG
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
     _countMutex.lock();
     _lockCount--;
 #endif
 
     // Release the lock.
     _mutex.unlock();
 
-#ifdef DENG2_DEBUG
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
     DENG2_ASSERT(_lockCount >= 0);
     _countMutex.unlock();
 #endif
 }
 
-#ifdef DENG2_DEBUG
+#ifdef DENG2_LOCKABLE_LOCK_COUNT
 bool Lockable::isLocked() const
 {
     _countMutex.lock();

doomsday/sdk/libcore/src/concurrency/task.cpp

@@ -39,7 +39,9 @@ void Task::run()
 
     // Cleanup.
     if (_pool) _pool->taskFinishedRunning(*this);
-    Log::disposeThreadLog();
+
+    // The thread's log is not disposed because task threads are pooled (by TaskPool)
+    // and the log object will be reused in future tasks.
 }
 
 } // namespace de

doomsday/sdk/libcore/src/core/id.cpp

@@ -22,12 +22,14 @@
 #include "de/Writer"
 #include "de/Reader"
 
+#include <atomic>
+
 namespace de {
 
 // The Id generator starts from one.
-Id::Type Id::_generator = 1;
+static std::atomic_uint idGenerator { 1 };
 
-Id::Id() : _id(_generator++)
+Id::Id() : _id(idGenerator++)
 {
     if (_id == None)
     {
@@ -91,9 +93,9 @@ void Id::operator << (Reader &from)
 
 void Id::resetGenerator(Id::Type largestKnownId)
 {
-    if (_generator <= largestKnownId)
+    if (idGenerator <= largestKnownId)
     {
-        _generator = largestKnownId + 1;
+        idGenerator = largestKnownId + 1;
     }
 }
 

doomsday/sdk/libcore/src/core/log.cpp

@@ -30,6 +30,7 @@
 #include <QMap>
 #include <QTextStream>
 #include <QThread>
+#include <QThreadStorage>
 #include <QStringList>
 
 namespace de {
@@ -52,26 +53,28 @@ static int const LINE_BREAKING_SECTION_LENGTH = 30;
 
 namespace internal {
 
+#if 0
 /**
  * @internal
  * The logs table is lockable so that multiple threads can access their logs at
  * the same time.
  */
-class Logs : public std::map<QThread *, Log *>, public Lockable
+class Logs : public Lockable, public QHash<QThread *, Log *>
 {
 public:
     Logs() {}
     ~Logs() {
-        DENG2_GUARD(this);
+        DENG2_GUARD_PTR(this);
         // The logs are owned by the logs table.
-        for (auto log : *this) delete log.second;
+        for (Log *log : *this) delete log;
     }
 };
+#endif
 
 } // namespace internal
 
 /// The logs table contains the log of each thread that uses logging.
-static std::unique_ptr<internal::Logs> logsPtr;
+//static std::unique_ptr<internal::Logs> logsPtr;
 
 /// Unused entry arguments are stored here in the pool.
 static FIFO<LogEntry::Arg> argPool;
@@ -718,50 +721,56 @@ LogEntry &Log::enter(duint32 metadata, String const &format, LogEntry::Args argu
     return *entry;
 }
 
-static internal::Logs &theLogs()
+/*static internal::Logs &theLogs()
 {
     if (logsPtr.get()) return *logsPtr;
     static Lockable lock;
     DENG2_GUARD(lock);
     if (!logsPtr.get()) logsPtr.reset(new internal::Logs);
     return *logsPtr;
-}
+}*/
+
+static QThreadStorage<Log> theLogs;
 
 Log &Log::threadLog()
 {
+    // Each thread has its own log.
+    //QThread *thread = QThread::currentThread();
+
+    return theLogs.localData();
+
+    /*if (!theLogs.hasLocalData())
+    {
+        theLogs.setLocalData(<#de::Log t#>)
     internal::Logs &logs = theLogs();
-
     DENG2_GUARD(logs);
-
-    // Each thread has its own log.
-    QThread *thread = QThread::currentThread();
-    auto found = logs.find(thread);
-    if (found == logs.end())
+    
+    auto found = logs.constFind(thread);
+    if (found == logs.constEnd())
     {
         // Create a new log.
-        Log* theLog = new Log;
-        logs[thread] = theLog;
+        Log *theLog = new Log;
+        logs.insert(thread, theLog);
         return *theLog;
     }
     else
     {
-        return *found->second;
-    }
+        return *found.value();
+    }*/
 }
 
 void Log::disposeThreadLog()
 {
-    internal::Logs &logs = theLogs();
-
+    /*internal::Logs &logs = theLogs();
     DENG2_GUARD(logs);
 
     QThread *thread = QThread::currentThread();
     auto found = logs.find(thread);
     if (found != logs.end())
     {
-        delete found->second;
+        delete found.value();
         logs.erase(found);
-    }
+    }*/
 }
 
 LogEntryStager::LogEntryStager(duint32 metadata, String const &format)

doomsday/sdk/libcore/src/data/record.cpp

@@ -37,6 +37,7 @@
 
 #include <QTextStream>
 #include <functional>
+#include <atomic>
 
 namespace de {
 
@@ -53,7 +54,7 @@ String const Record::VAR_NATIVE_SELF = "__self__";
  * Each record is given a unique identifier, so that serialized record
  * references can be tracked to their original target.
  */
-static duint32 recordIdCounter = 0;
+static std::atomic_uint recordIdCounter;
 
 DENG2_PIMPL(Record)
 , public Lockable