Safe darktable shutdown

src/common/darktable.c

@@ -2035,6 +2035,7 @@ void dt_cleanup()
 #endif
 
 #ifdef USE_LUA
+  // send the exit event to all the running scripts letting them know that darktable is ending
   dt_lua_finalize_early();
 #endif
 
@@ -2049,30 +2050,44 @@ void dt_cleanup()
     dt_ctl_switch_mode_to("");
     dt_dbus_destroy(darktable.dbus);
 
-    dt_lib_cleanup(darktable.lib);
-    free(darktable.lib);
+/* How is darktable shutdown working safely?
+1. dt_control_quit() is called via user request, it sets control->running anatomically
+     to DT_CONTROL_STATE_CLEANUP implying that dt_control_running() will not be TRUE any more.
+     It finally calls gtk_main_quit() so with current code we don't have gtk events after that.
+
+2. Quitting gtk also means **we are exactly here** to continue the shutdown. Anything requiring a
+     still active UI must be done before ...
+
+3. dt_control_shutdown() first waits for all threads to be joined, that means waiting for all pending
+     jobs to be done completely.
+
+4. So we have to ensure:
+   1) a full working software stack including image_cache, mipmap_cache and darktable.imageio to allow
+        processing the pixelpipe and full access to all images.
+   2) The pipeline processing uses access to gui related data - focus, active module ...
+        so make sure to avoid those too.
+   3) As lua events might be fired by the backthreads it's state mutex must still be unlocked.
+
+5. After dt_control_shutdown() has finished we are sure there are no background threads running any
+     more so we can safely close all mentioned subsystems and continue.
+*/
+    dt_control_shutdown(darktable.control);
   }
 #ifdef USE_LUA
   dt_lua_finalize();
 #endif
-  dt_view_manager_cleanup(darktable.view_manager);
-  free(darktable.view_manager);
-  darktable.view_manager = NULL;
-  // we can no longer call dt_gui_process_events after this point, as that will cause a segfault
-  // if some delayed event fires
 
-  dt_image_cache_cleanup(darktable.image_cache);
-  free(darktable.image_cache);
-  darktable.image_cache = NULL;
-  dt_mipmap_cache_cleanup(darktable.mipmap_cache);
-  free(darktable.mipmap_cache);
-  darktable.mipmap_cache = NULL;
   if(init_gui)
   {
+    dt_lib_cleanup(darktable.lib);
+    free(darktable.lib);
+    darktable.lib = NULL;
+    dt_view_manager_cleanup(darktable.view_manager);
+    free(darktable.view_manager);
+    darktable.view_manager = NULL;
     dt_imageio_cleanup(darktable.imageio);
     free(darktable.imageio);
     darktable.imageio = NULL;
-    dt_control_shutdown(darktable.control);
     dt_control_cleanup(darktable.control);
     free(darktable.control);
     darktable.control = NULL;
@@ -2082,6 +2097,14 @@ void dt_cleanup()
     darktable.gui = NULL;
   }
 
+  dt_image_cache_cleanup(darktable.image_cache);
+  free(darktable.image_cache);
+  darktable.image_cache = NULL;
+
+  dt_mipmap_cache_cleanup(darktable.mipmap_cache);
+  free(darktable.mipmap_cache);
+  darktable.mipmap_cache = NULL;
+
   dt_colorspaces_cleanup(darktable.color_profiles);
   dt_conf_cleanup(darktable.conf);
   free(darktable.conf);

src/common/datetime.c

@@ -233,7 +233,7 @@ GTimeSpan dt_datetime_now_to_gtimespan(void)
 void dt_datetime_exif_to_img(dt_image_t *img,
                              const char *exif)
 {
-  if(!exif) return;
+  if(!exif || !img) return;
   GDateTime *gdt = dt_datetime_exif_to_gdatetime(exif, darktable.utc_tz);
   if(gdt)
   {
@@ -247,7 +247,7 @@ gboolean dt_datetime_img_to_exif(char *exif,
                                  const size_t exif_size,
                                  const dt_image_t *img)
 {
-  return dt_datetime_gtimespan_to_exif(exif, exif_size, img->exif_datetime_taken);
+  return img ? dt_datetime_gtimespan_to_exif(exif, exif_size, img->exif_datetime_taken) : FALSE;
 }
 
 GDateTime *dt_datetime_exif_to_gdatetime(const char *exif,

src/common/exif.cc

@@ -2177,6 +2177,11 @@ static bool _exif_decode_exif_data(dt_image_t *img, Exiv2::ExifData &exifData)
 
 void dt_exif_apply_default_metadata(dt_image_t *img)
 {
+  if(!img)
+  {
+    dt_print(DT_DEBUG_ALWAYS, "[dt_exif_apply_default_metadata] failed as no img was provided");
+    return;
+  }
   if(dt_conf_get_bool("ui_last/import_apply_metadata")
      && !(img->job_flags & DT_IMAGE_JOB_NO_METADATA))
   {
@@ -2240,6 +2245,11 @@ gboolean dt_exif_read_from_blob(dt_image_t *img,
                                 uint8_t *blob,
                                 const int size)
 {
+  if(!img)
+  {
+    dt_print(DT_DEBUG_ALWAYS, "[exiv2 dt_exif_read_from_blob] failed as no img was provided");
+    return TRUE;
+  }
   try
   {
     Exiv2::ExifData exifData;
@@ -2321,6 +2331,11 @@ gboolean dt_exif_get_thumbnail(const char *path,
 gboolean dt_exif_read(dt_image_t *img,
                       const char *path)
 {
+  if(!img)
+  {
+    dt_print(DT_DEBUG_ALWAYS, "[dt_exif_read] failed as no img was provided");
+    return TRUE;
+  }
   // At least set 'datetime taken' to something useful in case there is
   // no Exif data in this file (pfm, png, ...)
   struct stat statbuf;
@@ -2778,7 +2793,7 @@ int dt_exif_read_blob(uint8_t **buf,
       // GPS data
       _remove_exif_geotag(exifData);
       const dt_image_t *cimg = dt_image_cache_get(darktable.image_cache, imgid, 'r');
-      if(!std::isnan(cimg->geoloc.longitude) && !std::isnan(cimg->geoloc.latitude))
+      if(cimg && !std::isnan(cimg->geoloc.longitude) && !std::isnan(cimg->geoloc.latitude))
       {
         exifData["Exif.GPSInfo.GPSVersionID"] = "02 02 00 00";
         exifData["Exif.GPSInfo.GPSLongitudeRef"] = (cimg->geoloc.longitude < 0) ? "W" : "E";
@@ -2799,7 +2814,7 @@ int dt_exif_read_blob(uint8_t **buf,
         g_free(long_str);
         g_free(lat_str);
       }
-      if(!std::isnan(cimg->geoloc.elevation))
+      if(cimg && !std::isnan(cimg->geoloc.elevation))
       {
         exifData["Exif.GPSInfo.GPSVersionID"] = "02 02 00 00";
         exifData["Exif.GPSInfo.GPSAltitudeRef"] = (cimg->geoloc.elevation < 0) ? "1" : "0";
@@ -3801,6 +3816,11 @@ gboolean dt_exif_xmp_read(dt_image_t *img,
                           const char *filename,
                           const int history_only)
 {
+  if(!img)
+  {
+    dt_print(DT_DEBUG_ALWAYS, "[dt_exif_xmp_read] failed as no img was provided for '%s'", filename);
+    return TRUE;
+  }
   // Exclude pfm to avoid stupid errors on the console
   const char *c = filename + strlen(filename) - 4;
   if(c >= filename && !strcmp(c, ".pfm")) return TRUE;

src/common/grouping.c

@@ -39,6 +39,7 @@ void dt_grouping_add_to_group(const dt_imgid_t group_id,
   dt_grouping_remove_from_group(image_id);
 
   dt_image_t *img = dt_image_cache_get(darktable.image_cache, image_id, 'w');
+  if(!img) return;
   img->group_id = group_id;
   dt_image_cache_write_release_info(darktable.image_cache, img,
                                     DT_IMAGE_CACHE_SAFE, "dt_grouping_add_to_group");
@@ -65,7 +66,7 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
   GList *imgs = NULL;
 
   const dt_image_t *img = dt_image_cache_get(darktable.image_cache, image_id, 'r');
-  const int img_group_id = img->group_id;
+  const dt_imgid_t img_group_id = img ? img->group_id : NO_IMGID;
   dt_image_cache_read_release(darktable.image_cache, img);
   if(img_group_id == image_id)
   {
@@ -84,10 +85,13 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
       if(!dt_is_valid_imgid(new_group_id))
         new_group_id = other_id;
       dt_image_t *other_img = dt_image_cache_get(darktable.image_cache, other_id, 'w');
-      other_img->group_id = new_group_id;
-      dt_image_cache_write_release_info(darktable.image_cache, other_img,
+      if(other_img)
+      {
+        other_img->group_id = new_group_id;
+        dt_image_cache_write_release_info(darktable.image_cache, other_img,
                                         DT_IMAGE_CACHE_SAFE, "dt_grouping_add_to_group");
-      imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
+        imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
+      }
     }
     sqlite3_finalize(stmt);
     if(dt_is_valid_imgid(new_group_id))
@@ -122,13 +126,15 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
   {
     // change the group_id for this image.
     dt_image_t *wimg = dt_image_cache_get(darktable.image_cache, image_id, 'w');
-    new_group_id = wimg->group_id;
-    wimg->group_id = image_id;
-    dt_image_cache_write_release_info(darktable.image_cache, wimg,
+    if(wimg)
+    {
+      new_group_id = wimg->group_id;
+      wimg->group_id = image_id;
+      dt_image_cache_write_release_info(darktable.image_cache, wimg,
                                       DT_IMAGE_CACHE_SAFE, "dt_grouping_add_to_group");
-    imgs = g_list_prepend(imgs, GINT_TO_POINTER(image_id));
-    // refresh also the group leader which may be alone now
-    imgs = g_list_prepend(imgs, GINT_TO_POINTER(img_group_id));
+      imgs = g_list_prepend(imgs, GINT_TO_POINTER(image_id));
+      // refresh also the group leader which may be alone now
+      imgs = g_list_prepend(imgs, GINT_TO_POINTER(img_group_id));
 #ifdef USE_LUA
     dt_lua_async_call_alien(dt_lua_event_trigger_wrapper,
       0, NULL, NULL,
@@ -138,6 +144,7 @@ dt_imgid_t dt_grouping_remove_from_group(const dt_imgid_t image_id)
       LUA_ASYNC_TYPENAME, "dt_lua_image_t", GINT_TO_POINTER(img_group_id),
       LUA_ASYNC_DONE);
 #endif
+    }
   }
   DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_IMAGE_INFO_CHANGED, imgs);
 
@@ -150,8 +157,10 @@ dt_imgid_t dt_grouping_change_representative(const dt_imgid_t image_id)
   sqlite3_stmt *stmt;
 
   dt_image_t *img = dt_image_cache_get(darktable.image_cache, image_id, 'r');
-  const dt_imgid_t group_id = img->group_id;
+  const dt_imgid_t group_id = img ? img->group_id : NO_IMGID;
   dt_image_cache_read_release(darktable.image_cache, img);
+ if(!dt_is_valid_imgid(group_id))
+    return group_id;
 
   GList *imgs = NULL;
   DT_DEBUG_SQLITE3_PREPARE_V2(dt_database_get(darktable.db), "SELECT id FROM main.images WHERE group_id = ?1", -1,
@@ -161,11 +170,14 @@ dt_imgid_t dt_grouping_change_representative(const dt_imgid_t image_id)
   {
     const dt_imgid_t other_id = sqlite3_column_int(stmt, 0);
     dt_image_t *other_img = dt_image_cache_get(darktable.image_cache, other_id, 'w');
-    other_img->group_id = image_id;
-    dt_image_cache_write_release_info(darktable.image_cache, other_img,
+    if(other_img)
+    {
+      other_img->group_id = image_id;
+      dt_image_cache_write_release_info(darktable.image_cache, other_img,
                                       DT_IMAGE_CACHE_SAFE,
                                       "dt_grouping_change_representative");
-    imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
+      imgs = g_list_prepend(imgs, GINT_TO_POINTER(other_id));
+    }
   }
   sqlite3_finalize(stmt);
   DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_IMAGE_INFO_CHANGED, imgs);

src/common/image.c

@@ -851,43 +851,44 @@ void dt_image_update_final_size(const dt_imgid_t imgid)
                                     &ww, &hh);
 
     dt_image_t *imgtmp = dt_image_cache_get(darktable.image_cache, imgid, 'w');
-    if(!imgtmp || (ww == imgtmp->final_width && hh == imgtmp->final_height))
-    {
-      dt_cache_release(&darktable.image_cache->cache, imgtmp->cache_entry);
-    }
-    else
+    if(imgtmp)
     {
+      const gboolean changed = (ww != imgtmp->final_width) || (hh != imgtmp->final_height);
       imgtmp->final_width = ww;
       imgtmp->final_height = hh;
       dt_image_cache_write_release(darktable.image_cache, imgtmp, DT_IMAGE_CACHE_RELAXED);
-      DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_METADATA_UPDATE);
-      DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_DEVELOP_IMAGE_CHANGED);
-      dt_print(DT_DEBUG_PIPE, "updated final size for ID=%i to %ix%i", imgid, ww, hh);
+      if(changed)
+      {
+        dt_print(DT_DEBUG_PIPE, "updated final size for ID=%i, updated to %ix%i", imgid, ww, hh);
+        DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_METADATA_UPDATE);
+        DT_CONTROL_SIGNAL_RAISE(DT_SIGNAL_DEVELOP_IMAGE_CHANGED);
+      }
     }
   }
 }
 
 gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height)
 {
+  if(!dt_is_valid_imgid(imgid)) return TRUE;
   // get the img strcut
-  dt_image_t *imgtmp = dt_image_cache_get(darktable.image_cache, imgid, 'r');
-  dt_image_t img = *imgtmp;
-  dt_image_cache_read_release(darktable.image_cache, imgtmp);
-  if(!imgtmp)
+  dt_image_t *timg = dt_image_cache_get(darktable.image_cache, imgid, 'r');
+  if(!timg)
   {
     *width = 0;
     *height = 0;
     return FALSE;
   }
 
   // if we already have computed them
-  if(img.final_height > 0 && img.final_width > 0)
+  const gboolean available = timg->final_height > 0 && timg->final_width > 0;
+  if(available)
   {
-    *width = img.final_width;
-    *height = img.final_height;
+    *width = timg->final_width;
+    *height = timg->final_height;
     dt_print(DT_DEBUG_PIPE, "[dt_image_get_final_size] for ID=%i from cache %ix%i", imgid, *width, *height);
-    return FALSE;
   }
+  dt_image_cache_read_release(darktable.image_cache, timg);
+  if(available) return FALSE;
 
   // we have to do the costly pipe run to get the final image size
   dt_print(DT_DEBUG_PIPE, "[dt_image_get_final_size] calculate it for ID=%i", imgid);
@@ -896,8 +897,9 @@ gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height
   dt_dev_load_image(&dev, imgid);
 
   dt_dev_pixelpipe_t pipe;
-  int wd = dev.image_storage.width, ht = dev.image_storage.height;
-  gboolean res = dt_dev_pixelpipe_init_dummy(&pipe, wd, ht);
+  int wd = dev.image_storage.width;
+  int ht = dev.image_storage.height;
+  const gboolean res = dt_dev_pixelpipe_init_dummy(&pipe, wd, ht);
   if(res)
   {
     // set mem pointer to 0, won't be used.
@@ -909,16 +911,17 @@ gboolean dt_image_get_final_size(const dt_imgid_t imgid, int *width, int *height
                                     &pipe.processed_height);
     wd = pipe.processed_width;
     ht = pipe.processed_height;
-    res = TRUE;
     dt_dev_pixelpipe_cleanup(&pipe);
   }
   dt_dev_cleanup(&dev);
 
-  imgtmp = dt_image_cache_get(darktable.image_cache, imgid, 'w');
-  imgtmp->final_width = *width = wd;
-  imgtmp->final_height = *height = ht;
-  dt_image_cache_write_release(darktable.image_cache, imgtmp, DT_IMAGE_CACHE_RELAXED);
-
+  dt_image_t * imgwr = dt_image_cache_get(darktable.image_cache, imgid, 'w');
+  if(imgwr)
+  {
+    imgwr->final_width = *width = wd;
+    imgwr->final_height = *height = ht;
+    dt_image_cache_write_release(darktable.image_cache, imgwr, DT_IMAGE_CACHE_RELAXED);
+  }
   return res;
 }
 
@@ -1097,11 +1100,11 @@ void dt_image_set_raw_aspect_ratio(const dt_imgid_t imgid)
       image->aspect_ratio = (float )image->p_width / (float )(MAX(1, image->p_height));
     else
       image->aspect_ratio = (float )image->p_height / (float )(MAX(1, image->p_width));
-  }
-  /* store */
-  dt_image_cache_write_release_info(darktable.image_cache, image,
+    /* store */
+    dt_image_cache_write_release_info(darktable.image_cache, image,
                                     DT_IMAGE_CACHE_SAFE,
                                     "dt_image_set_raw_aspect_ratio");
+  }
 }
 
 void dt_image_set_aspect_ratio_to(const dt_imgid_t imgid,
@@ -1160,17 +1163,19 @@ void dt_image_reset_aspect_ratio(const dt_imgid_t imgid, const gboolean raise)
   dt_image_t *image = dt_image_cache_get(darktable.image_cache, imgid, 'w');
 
   /* set image aspect ratio */
-  if(image) image->aspect_ratio = 0.f;
-
-  /* store in db, but don't synch XMP */
-  dt_image_cache_write_release_info(darktable.image_cache, image,
+  if(image)
+  {
+    image->aspect_ratio = 0.f;
+    /* store in db, but don't synch XMP */
+    dt_image_cache_write_release_info(darktable.image_cache, image,
                                     DT_IMAGE_CACHE_RELAXED,
                                     "dt_image_reset_aspect_ratio");
 
-  if(image && raise && darktable.collection->params.sorts[DT_COLLECTION_SORT_ASPECT_RATIO])
+  if(raise && darktable.collection->params.sorts[DT_COLLECTION_SORT_ASPECT_RATIO])
     dt_collection_update_query(darktable.collection, DT_COLLECTION_CHANGE_RELOAD,
                                DT_COLLECTION_PROP_ASPECT_RATIO,
                                g_list_prepend(NULL, GINT_TO_POINTER(imgid)));
+  }
 }
 
 float dt_image_set_aspect_ratio(const dt_imgid_t imgid, const gboolean raise)