Fix several issues found with ThreadSanitizer

laf

@@ -1 +1 @@
-Subproject commit 9c6ecadd9cbcfd2b87b7cfa61e01466013da3797
+Subproject commit 85259c5421f06f55143cba7c69708f01e0fc026b

src/app/check_update.h

@@ -1,4 +1,5 @@
 // Aseprite
+// Copyright (C) 2020  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -14,6 +15,7 @@
 #include "ui/timer.h"
 #include "updater/check_update.h"
 
+#include <atomic>
 #include <memory>
 
 namespace app {
@@ -47,7 +49,7 @@ namespace app {
     std::unique_ptr<base::thread> m_thread;
     std::unique_ptr<CheckUpdateBackgroundJob> m_bgJob;
     bool m_doCheck;
-    bool m_received;
+    std::atomic<bool> m_received;
 
     // Mini-stats
     int m_inits;

src/app/crash/backup_observer.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019  Igara Studio S.A.
+// Copyright (C) 2019-2020  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -13,6 +13,7 @@
 #include "app/doc_observer.h"
 #include "app/docs_observer.h"
 
+#include <atomic>
 #include <condition_variable>
 #include <mutex>
 #include <thread>
@@ -48,7 +49,7 @@ namespace crash {
     Context* m_ctx;
     std::vector<Doc*> m_documents;
     std::vector<Doc*> m_closedDocs;
-    bool m_done;
+    std::atomic<bool> m_done;
 
     std::mutex m_mutex;
 

src/app/file_system.cpp

@@ -80,8 +80,8 @@ public:
   unsigned int m_version;
   bool m_removed;
   mutable bool m_is_folder;
-  double m_thumbnailProgress;
-  os::Surface* m_thumbnail;
+  std::atomic<double> m_thumbnailProgress;
+  std::atomic<os::Surface*> m_thumbnail;
 #ifdef _WIN32
   LPITEMIDLIST m_pidl;            // relative to parent
   LPITEMIDLIST m_fullpidl;        // relative to the Desktop folder
@@ -593,9 +593,9 @@ os::Surface* FileItem::getThumbnail()
 
 void FileItem::setThumbnail(os::Surface* thumbnail)
 {
-  if (m_thumbnail)
-    m_thumbnail->dispose();
-  m_thumbnail = thumbnail;
+  auto old = m_thumbnail.exchange(thumbnail);
+  if (old)
+    old->dispose();
 }
 
 FileItem::FileItem(FileItem* parent)
@@ -621,8 +621,8 @@ FileItem::~FileItem()
 {
   FS_TRACE("FS: Destroying FileItem() with parent %p\n", m_parent);
 
-  if (m_thumbnail)
-    m_thumbnail->dispose();
+  if (auto ptr = m_thumbnail.load())
+    ptr->dispose();
 
 #ifdef _WIN32
   if (m_fullpidl && m_fullpidl != m_pidl) {

src/app/resource_finder.cpp

@@ -28,8 +28,8 @@ namespace app {
 
 ResourceFinder::ResourceFinder(bool log)
   : m_log(log)
+  , m_current(-1)
 {
-  m_current = -1;
 }
 
 const std::string& ResourceFinder::filename() const

src/app/resource_finder.h

@@ -1,5 +1,5 @@
 // Aseprite
-// Copyright (C) 2019  Igara Studio S.A.
+// Copyright (C) 2019-2020  Igara Studio S.A.
 // Copyright (C) 2001-2018  David Capello
 //
 // This program is distributed under the terms of
@@ -12,6 +12,7 @@
 #include "base/disable_copying.h"
 #include "base/paths.h"
 
+#include <atomic>
 #include <string>
 
 namespace app {
@@ -64,7 +65,7 @@ namespace app {
   private:
     bool m_log;
     base::paths m_paths;
-    int m_current;
+    std::atomic<int> m_current;
     std::string m_default;
 
     DISABLE_COPYING(ResourceFinder);

src/app/task.cpp

@@ -29,6 +29,7 @@ Task::~Task()
 
 void Task::run(base::task::func_t&& func)
 {
+  std::lock_guard<std::mutex> lock(m_token_mutex);
   m_task.on_execute(std::move(func));
   m_token = &m_task.start(tasks_pool);
 }

src/app/task.h

@@ -11,6 +11,7 @@
 #include "base/task.h"
 
 #include <functional>
+#include <mutex>
 
 namespace app {
 
@@ -33,6 +34,7 @@ namespace app {
     }
 
     bool canceled() const {
+      std::lock_guard<std::mutex> lock(m_token_mutex);
       if (m_token)
         return m_token->canceled();
       else
@@ -40,6 +42,7 @@ namespace app {
     }
 
     float progress() const {
+      std::lock_guard<std::mutex> lock(m_token_mutex);
       if (m_token)
         return m_token->progress();
       else
@@ -47,17 +50,20 @@ namespace app {
     }
 
     void cancel() {
+      std::lock_guard<std::mutex> lock(m_token_mutex);
       if (m_token)
         m_token->cancel();
     }
 
     void set_progress(float progress) {
+      std::lock_guard<std::mutex> lock(m_token_mutex);
       if (m_token)
         m_token->set_progress(progress);
     }
 
   private:
     base::task m_task;
+    mutable std::mutex m_token_mutex;
     base::task_token* m_token;
   };
 

src/app/thumbnail_generator.cpp

@@ -31,6 +31,7 @@
 #include "render/render.h"
 
 #include <algorithm>
+#include <atomic>
 #include <memory>
 #include <thread>
 
@@ -167,7 +168,10 @@ private:
           thumbnailImage.get(), palette.get(), thumbnail,
           0, 0, 0, 0, thumbnailImage->width(), thumbnailImage->height());
 
-        m_item.fileitem->setThumbnail(thumbnail);
+        {
+          base::scoped_lock lock(m_mutex);
+          m_item.fileitem->setThumbnail(thumbnail);
+        }
       }
 
       THUMB_TRACE("FOP done with thumbnail: %s %s\n",
@@ -176,7 +180,10 @@ private:
 
       // Reset the m_item (first the fileitem so this worker is not
       // associated to this fileitem anymore, and then the FileOp).
-      m_item.fileitem = nullptr;
+      {
+        base::scoped_lock lock(m_mutex);
+        m_item.fileitem = nullptr;
+      }
     }
     catch (const std::exception& e) {
       m_fop->setError("Error loading file:\n%s", e.what());
@@ -205,7 +212,7 @@ private:
   app::ThumbnailGenerator::Item m_item;
   FileOp* m_fop;
   mutable base::mutex m_mutex;
-  bool m_isDone;
+  std::atomic<bool> m_isDone;
   base::thread m_thread;
 };
 

third_party/tinyxml/CMakeLists.txt

@@ -1,8 +1,12 @@
 # ASEPRITE
+# Copyright (C) 2020  Igara Studio S.A.
 # Copyright (C) 2001-2013  David Capello
 
 add_library(tinyxml
-  tinystr.cpp
   tinyxml.cpp
   tinyxmlerror.cpp
   tinyxmlparser.cpp)
+
+# Use std::string instead of TiXmlString (we've found some threading
+# issues related to TiXmlString::nullrep_)
+target_compile_definitions(tinyxml PUBLIC TIXML_USE_STL)