[Bf-blender-cvs] [88f8b01e669] blender-v3.0-release: Fix T93691: Crash when loading custom thumbnail in custom library

[Julian Eisel]

Commit: 88f8b01e669db8877bed87e1d2fc9ad5e4d885a0
Author: Julian Eisel
Date:   Thu Dec 9 15:30:21 2021 +0100
Branches: blender-v3.0-release
https://developer.blender.org/rB88f8b01e669db8877bed87e1d2fc9ad5e4d885a0

Fix T93691: Crash when loading custom thumbnail in custom library

This was an issue with the mixed list of external assets and assets from
the current file. When closing the File Browser to select the custom
preview image, the assets from the current file would be cleared for
reread, to make sure we display up-to-date file data. That is because
the workspace of the temporary File Browser was deleted, causing a
change in the file data (main data-base). The reread would happen in a
background thread, meaning it might not finish before the custom preview
operator runs and queries the active asset. So the preview operator
would get the wrong active asset from context.

Two fixes were needed:
* Make sure current file data is reread before the operator runs, by
  doing this partial rereading on the main thread.
* Ensure the asset list (in fact file list) order stays consistent over
  rereads. If multiple assets with the same name were shown, the
  operator might also have gotten the wrong asset, also leading to a
  crash.

Additionally the file operation handler should probably poll before
executing, to fail gracefully at least (not crash).

===================================================================

M	source/blender/editors/space_file/filelist.c

===================================================================

diff --git a/source/blender/editors/space_file/filelist.c b/source/blender/editors/space_file/filelist.c
index 3e7a831691c..68984da0584 100644
--- a/source/blender/editors/space_file/filelist.c
+++ b/source/blender/editors/space_file/filelist.c
@@ -508,6 +508,53 @@ static int compare_apply_inverted(int val, const struct FileSortData *sort_data)
   return sort_data->inverted ? -val : val;
 }
 
+/**
+ * If all relevant characteristics match (e.g. the file type when sorting by file types), this
+ * should be used as tiebreaker. It makes sure there's a well defined sorting even in such cases.
+ *
+ * Multiple files with the same name can appear with recursive file loading and/or when displaying
+ * IDs of different types, so these cases need to be handled.
+ *
+ * 1) Sort files by name using natural sorting.
+ * 2) If not possible (file names match) and both represent local IDs, sort by ID-type.
+ * 3) If not possible and only one is a local ID, place files representing local IDs first.
+ *
+ * TODO (not actually implemented, but should be):
+ * 4) If no file represents a local ID, sort by file path, so that files higher up the file system
+ *    hierarchy are placed first.
+ */
+static int compare_tiebreaker(const FileListInternEntry *entry1, const FileListInternEntry *entry2)
+{
+  /* Case 1. */
+  {
+    const int order = BLI_strcasecmp_natural(entry1->name, entry2->name);
+    if (order) {
+      return order;
+    }
+  }
+
+  /* Case 2. */
+  if (entry1->local_data.id && entry2->local_data.id) {
+    if (entry1->blentype < entry2->blentype) {
+      return -1;
+    }
+    if (entry1->blentype > entry2->blentype) {
+      return 1;
+    }
+  }
+  /* Case 3. */
+  {
+    if (entry1->local_data.id && !entry2->local_data.id) {
+      return -1;
+    }
+    if (!entry1->local_data.id && entry2->local_data.id) {
+      return 1;
+    }
+  }
+
+  return 0;
+}
+
 /**
  * Handles inverted sorting itself (currently there's nothing to invert), so if this returns non-0,
  * it should be used as-is and not inverted.
@@ -568,17 +615,13 @@ static int compare_name(void *user_data, const void *a1, const void *a2)
   const FileListInternEntry *entry1 = a1;
   const FileListInternEntry *entry2 = a2;
   const struct FileSortData *sort_data = user_data;
-  char *name1, *name2;
-  int ret;
 
+  int ret;
   if ((ret = compare_direntry_generic(entry1, entry2))) {
     return ret;
   }
 
-  name1 = entry1->name;
-  name2 = entry2->name;
-
-  return compare_apply_inverted(BLI_strcasecmp_natural(name1, name2), sort_data);
+  return compare_apply_inverted(compare_tiebreaker(entry1, entry2), sort_data);
 }
 
 static int compare_date(void *user_data, const void *a1, const void *a2)
@@ -586,10 +629,9 @@ static int compare_date(void *user_data, const void *a1, const void *a2)
   const FileListInternEntry *entry1 = a1;
   const FileListInternEntry *entry2 = a2;
   const struct FileSortData *sort_data = user_data;
-  char *name1, *name2;
   int64_t time1, time2;
-  int ret;
 
+  int ret;
   if ((ret = compare_direntry_generic(entry1, entry2))) {
     return ret;
   }
@@ -603,10 +645,7 @@ static int compare_date(void *user_data, const void *a1, const void *a2)
     return compare_apply_inverted(-1, sort_data);
   }
 
-  name1 = entry1->name;
-  name2 = entry2->name;
-
-  return compare_apply_inverted(BLI_strcasecmp_natural(name1, name2), sort_data);
+  return compare_apply_inverted(compare_tiebreaker(entry1, entry2), sort_data);
 }
 
 static int compare_size(void *user_data, const void *a1, const void *a2)
@@ -614,7 +653,6 @@ static int compare_size(void *user_data, const void *a1, const void *a2)
   const FileListInternEntry *entry1 = a1;
   const FileListInternEntry *entry2 = a2;
   const struct FileSortData *sort_data = user_data;
-  char *name1, *name2;
   uint64_t size1, size2;
   int ret;
 
@@ -631,10 +669,7 @@ static int compare_size(void *user_data, const void *a1, const void *a2)
     return compare_apply_inverted(-1, sort_data);
   }
 
-  name1 = entry1->name;
-  name2 = entry2->name;
-
-  return compare_apply_inverted(BLI_strcasecmp_natural(name1, name2), sort_data);
+  return compare_apply_inverted(compare_tiebreaker(entry1, entry2), sort_data);
 }
 
 static int compare_extension(void *user_data, const void *a1, const void *a2)
@@ -642,7 +677,6 @@ static int compare_extension(void *user_data, const void *a1, const void *a2)
   const FileListInternEntry *entry1 = a1;
   const FileListInternEntry *entry2 = a2;
   const struct FileSortData *sort_data = user_data;
-  char *name1, *name2;
   int ret;
 
   if ((ret = compare_direntry_generic(entry1, entry2))) {
@@ -690,10 +724,7 @@ static int compare_extension(void *user_data, const void *a1, const void *a2)
     }
   }
 
-  name1 = entry1->name;
-  name2 = entry2->name;
-
-  return compare_apply_inverted(BLI_strcasecmp_natural(name1, name2), sort_data);
+  return compare_apply_inverted(compare_tiebreaker(entry1, entry2), sort_data);
 }
 
 void filelist_sort(struct FileList *filelist)
@@ -3945,7 +3976,13 @@ void filelist_readjob_start(FileList *filelist, const int space_notifier, const
   /* Init even for single threaded execution. Called functions use it. */
   BLI_mutex_init(&flrj->lock);
 
-  if (filelist->tags & FILELIST_TAGS_NO_THREADS) {
+  /* The file list type may not support threading so execute immediately. Same when only rereading
+   * #Main data (which we do quite often on changes to #Main, since it's the easiest and safest way
+   * to ensure the displayed data is up to date), because some operations executing right after
+   * main data changed may need access to the ID files (see T93691). */
+  const bool no_threads = (filelist->tags & FILELIST_TAGS_NO_THREADS) || flrj->only_main_data;
+
+  if (no_threads) {
     short dummy_stop = false;
     short dummy_do_update = false;
     float dummy_progress = 0.0f;