gui: thread sanitizer issues

hanatos · Jan 12, 2024 · 903b275 · 903b275
1 parent 1ebf3ea
commit 903b275
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 27 deletions.
diff --git a/src/core/fs.h b/src/core/fs.h
@@ -378,7 +378,8 @@ fs_expand_import_filename(
 #warning "port me!"
 #else
   time_t t = time(0);
-  struct tm *tm = localtime(&t);
+  struct tm result;
+  struct tm *tm = localtime_r(&t, &result);
   strftime(date, sizeof(date), "%Y%m%d", tm);
   strftime(yyyy, sizeof(yyyy), "%Y", tm);
 #endif
@@ -405,7 +406,8 @@ fs_expand_export_filename(
 #warning "port me!"
 #else
   time_t t = time(0);
-  struct tm *tm = localtime(&t);
+  struct tm result;
+  struct tm *tm = localtime_r(&t, &result);
   strftime(date, sizeof(date), "%Y%m%d", tm);
   strftime(yyyy, sizeof(yyyy), "%Y", tm);
 #endif

diff --git a/src/core/threads.c b/src/core/threads.c
@@ -47,8 +47,8 @@ threads_task_t;
 
 typedef struct threads_t
 {
-  uint32_t num_threads;
-  int      shutdown;
+  uint32_t        num_threads;
+  atomic_int      shutdown;
 
   // worker list
   pthread_t      *worker;

diff --git a/src/db/thumbnails.c b/src/db/thumbnails.c
@@ -307,13 +307,10 @@ static void thread_free_coll(void *arg)
 {
   // task is done, every thread will call this
   cache_coll_job_t *j = arg;
-  // only first thread frees 
-  if(j->gid == 0)
-  {
-    pthread_mutex_destroy(&j->mutex_storage);
-    free(j->coll);
-    free(j);
-  }
+  // only first thread destroys mutex
+  if(j->gid == 0) pthread_mutex_destroy(&j->mutex_storage);
+  free(j->coll);
+  free(j);
 }
 
 void
@@ -366,26 +363,28 @@ dt_thumbnails_cache_list(
 
   uint32_t *collection = malloc(sizeof(uint32_t) * imgid_cnt);
   memcpy(collection, imgid, sizeof(uint32_t) * imgid_cnt); // take copy because this thing changes
-  cache_coll_job_t *job = malloc(sizeof(cache_coll_job_t)*DT_THUMBNAILS_THREADS);
+  cache_coll_job_t *job0 = 0;
   int taskid = -1;
   for(int k=0;k<DT_THUMBNAILS_THREADS;k++)
   {
+    cache_coll_job_t *job = malloc(sizeof(cache_coll_job_t));
     if(k == 0)
     {
-      job[0] = (cache_coll_job_t) {
+      *job = (cache_coll_job_t) {
         .stamp = tn->job_timestamp,
         .coll  = collection,
         .gid   = k,
         .tn    = tn,
         .db    = db,
         .ufn   = updatefn,
       };
-      threads_mutex_init(&job[0].mutex_storage, 0);
-      job[0].mutex = &job[0].mutex_storage;
+      threads_mutex_init(&job->mutex_storage, 0);
+      job->mutex = &job->mutex_storage;
+      job0 = job;
     }
-    else job[k] = (cache_coll_job_t) {
+    else *job = (cache_coll_job_t) {
       .stamp = tn->job_timestamp,
-      .mutex = &job[0].mutex_storage,
+      .mutex = &job0->mutex_storage,
       .coll  = collection,
       .gid   = k,
       .tn    = tn,
@@ -398,7 +397,7 @@ dt_thumbnails_cache_list(
         "thumb",
         imgid_cnt,
         taskid,
-        job+k,
+        job,
         thread_work_coll,
         thread_free_coll);
     if(taskid < 0) return VK_INCOMPLETE;

diff --git a/src/gui/render_lighttable.cc b/src/gui/render_lighttable.cc
@@ -259,6 +259,7 @@ struct export_job_t
   char basename[1000];
   uint8_t *pdata;
   std::atomic_uint abort;
+  std::atomic_uint state; // 0 idle, 1 started, 2 cleaned
   int taskid;
   dt_graph_t graph;
 };
@@ -268,6 +269,7 @@ void export_job_cleanup(void *arg)
   free(j->sel);
   free(j->pdata);
   dt_graph_cleanup(&j->graph);
+  j->state = 2;
 }
 void export_job_work(uint32_t item, void *arg)
 {
@@ -309,6 +311,7 @@ int export_job(
     dt_export_widget_t *w)
 {
   j->abort = 0;
+  j->state = 1;
   j->overwrite = w->overwrite;
   if(vkdt.db.selection_cnt <= 0)
   {
@@ -787,7 +790,8 @@ void render_lighttable_right_panel(int hotkey)
     for(int k=0;k<NUM_JOBS;k++)
     { // list of jobs to export stuff simultaneously
       ImGui::PushID(k);
-      if(job[k].cnt == 0)
+      if(job[k].state == 2) job[k].state = 0; // reset
+      if(job[k].state == 0)
       { // idle job
         if(num_idle++)
         { // show at max one idle job
@@ -805,17 +809,11 @@ void render_lighttable_right_panel(int hotkey)
         if(ImGui::Button("abort")) job[k].abort = 1;
         ImGui::SameLine();
         ImGui::ProgressBar(threads_task_progress(job[k].taskid), ImVec2(-1, 0));
+        // technically a race condition on frame_cnt being inited by graph
+        // loading during the async job. do we care?
         if(job[k].graph.frame_cnt > 1)
           ImGui::ProgressBar(job[k].graph.frame / (float)job[k].graph.frame_cnt, ImVec2(-1, 0));
       }
-      else
-      { // done/aborted
-        if(ImGui::Button(job[k].abort ? "aborted" : "done"))
-        { // reset
-          memset(job+k, 0, sizeof(export_job_t));
-        }
-        if(ImGui::IsItemHovered()) dt_gui_set_tooltip("click to reset");
-      }
       ImGui::PopID();
     }
 #undef NUM_JOBS