Fix T49829: Removal of custom icon previews during add-on unregister crashes Blender.

Issue was happening when removal of custom icons was done while they
were still being rendered by preview job.

Now add a 'deffered deletion' system, to prevent main thread to delete
preview image until loading thread is done with them.

Note that ideally, calling `ED_preview_kill_jobs()` on custom icon
removal would have been simpler, but we don't have easy access to
context here...

6 files changed, 54 insertions, 14 deletions

diff --git a/source/blender/blenkernel/BKE_icons.h b/source/blender/blenkernel/BKE_icons.h
index efef8d4be78..6944c5ccd28 100644
--- a/source/blender/blenkernel/BKE_icons.h
+++ b/source/blender/blenkernel/BKE_icons.h
@@ -114,6 +114,7 @@ struct PreviewImage *BKE_previewimg_cached_thumbnail_read(
         const char *name, const char *path, const int source, bool force_update);
 
 void BKE_previewimg_cached_release(const char *name);
+void BKE_previewimg_cached_release_pointer(struct PreviewImage *prv);
 
 #define ICON_RENDER_DEFAULT_HEIGHT 32
 
diff --git a/source/blender/blenkernel/intern/icons.c b/source/blender/blenkernel/intern/icons.c
index 2d5b15c8f9d..7669c4ba112 100644
--- a/source/blender/blenkernel/intern/icons.c
+++ b/source/blender/blenkernel/intern/icons.c
@@ -143,7 +143,7 @@ static PreviewImage *previewimg_create_ex(size_t deferred_data_size)
 	memset(prv_img, 0, sizeof(*prv_img));  /* leave deferred data dirty */
 
 	if (deferred_data_size) {
-		prv_img->use_deferred = true;
+		prv_img->tag |= PRV_TAG_DEFFERED;
 	}
 
 	for (i = 0; i < NUM_ICON_SIZES; ++i) {
@@ -355,11 +355,14 @@ PreviewImage *BKE_previewimg_cached_thumbnail_read(
 	return prv;
 }
 
-void BKE_previewimg_cached_release(const char *name)
+void BKE_previewimg_cached_release_pointer(PreviewImage *prv)
 {
-	PreviewImage *prv = BLI_ghash_popkey(gCachedPreviews, name, MEM_freeN);
-
 	if (prv) {
+		if (prv->tag & PRV_TAG_DEFFERED_RENDERING) {
+			/* We cannot delete the preview while it is being loaded in another thread... */
+			prv->tag |= PRV_TAG_DEFFERED_DELETE;
+			return;
+		}
 		if (prv->icon_id) {
 			BKE_icon_delete(prv->icon_id);
 		}
@@ -367,11 +370,18 @@ void BKE_previewimg_cached_release(const char *name)
 	}
 }
 
+void BKE_previewimg_cached_release(const char *name)
+{
+	PreviewImage *prv = BLI_ghash_popkey(gCachedPreviews, name, MEM_freeN);
+
+	BKE_previewimg_cached_release_pointer(prv);
+}
+
 /** Handle deferred (lazy) loading/generation of preview image, if needed.
  * For now, only used with file thumbnails. */
 void BKE_previewimg_ensure(PreviewImage *prv, const int size)
 {
-	if (prv->use_deferred) {
+	if ((prv->tag & PRV_TAG_DEFFERED) != 0) {
 		const bool do_icon = ((size == ICON_SIZE_ICON) && !prv->rect[ICON_SIZE_ICON]);
 		const bool do_preview = ((size == ICON_SIZE_PREVIEW) && !prv->rect[ICON_SIZE_PREVIEW]);
 
diff --git a/source/blender/blenloader/intern/readfile.c b/source/blender/blenloader/intern/readfile.c
index c1da78d3877..41b275751d1 100644
--- a/source/blender/blenloader/intern/readfile.c
+++ b/source/blender/blenloader/intern/readfile.c
@@ -2147,6 +2147,7 @@ static PreviewImage *direct_link_preview_image(FileData *fd, PreviewImage *old_p
 			prv->gputexture[i] = NULL;
 		}
 		prv->icon_id = 0;
+		prv->tag = 0;
 	}
 	
 	return prv;
diff --git a/source/blender/editors/interface/interface_icons.c b/source/blender/editors/interface/interface_icons.c
index 22a450d2523..ff9d2840e9c 100644
--- a/source/blender/editors/interface/interface_icons.c
+++ b/source/blender/editors/interface/interface_icons.c
@@ -1088,7 +1088,7 @@ void ui_icon_ensure_deferred(const bContext *C, const int icon_id, const bool bi
 					if (prv) {
 						const int size = big ? ICON_SIZE_PREVIEW : ICON_SIZE_ICON;
 
-						if (id || prv->use_deferred) {
+						if (id || (prv->tag & PRV_TAG_DEFFERED) != 0) {
 							ui_id_preview_image_render_size(C, NULL, id, prv, size, true);
 						}
 					}
diff --git a/source/blender/editors/render/render_preview.c b/source/blender/editors/render/render_preview.c
index ddbf59b2cf7..87c08dc6583 100644
--- a/source/blender/editors/render/render_preview.c
+++ b/source/blender/editors/render/render_preview.c
@@ -1080,13 +1080,19 @@ static void icon_preview_add_size(IconPreview *ip, unsigned int *rect, int sizex
 static void icon_preview_startjob_all_sizes(void *customdata, short *stop, short *do_update, float *progress)
 {
 	IconPreview *ip = (IconPreview *)customdata;
-	IconPreviewSize *cur_size = ip->sizes.first;
+	IconPreviewSize *cur_size;
 	const bool use_new_shading = BKE_scene_use_new_shading_nodes(ip->scene);
 
-	while (cur_size) {
+	for (cur_size = ip->sizes.first; cur_size; cur_size = cur_size->next) {
 		PreviewImage *prv = ip->owner;
+
+		if (prv->tag & PRV_TAG_DEFFERED_DELETE) {
+			/* Non-thread-protected reading is not an issue here. */
+			continue;
+		}
+
 		ShaderPreview *sp = MEM_callocN(sizeof(ShaderPreview), "Icon ShaderPreview");
-		const bool is_render = !prv->use_deferred;
+		const bool is_render = !(prv->tag & PRV_TAG_DEFFERED);
 
 		/* construct shader preview from image size and previewcustomdata */
 		sp->scene = ip->scene;
@@ -1117,8 +1123,6 @@ static void icon_preview_startjob_all_sizes(void *customdata, short *stop, short
 
 		common_preview_startjob(sp, stop, do_update, progress);
 		shader_preview_free(sp);
-
-		cur_size = cur_size->next;
 	}
 }
 
@@ -1147,6 +1151,15 @@ static void icon_preview_endjob(void *customdata)
 		}
 #endif
 	}
+
+	if (ip->owner) {
+		PreviewImage *prv_img = ip->owner;
+		prv_img->tag &= ~PRV_TAG_DEFFERED_RENDERING;
+		if (prv_img->tag & PRV_TAG_DEFFERED_DELETE) {
+			BLI_assert(prv_img->tag & PRV_TAG_DEFFERED);
+			BKE_previewimg_cached_release_pointer(prv_img);
+		}
+	}
 }
 
 static void icon_preview_free(void *customdata)
@@ -1205,6 +1218,14 @@ void ED_preview_icon_job(const bContext *C, void *owner, ID *id, unsigned int *r
 
 	icon_preview_add_size(ip, rect, sizex, sizey);
 
+	/* Special threading hack: warn main code that this preview is being rendered and cannot be freed... */
+	{
+		PreviewImage *prv_img = owner;
+		if (prv_img->tag & PRV_TAG_DEFFERED) {
+			prv_img->tag |= PRV_TAG_DEFFERED_RENDERING;
+		}
+	}
+
 	/* setup job */
 	WM_jobs_customdata_set(wm_job, ip, icon_preview_free);
 	WM_jobs_timer(wm_job, 0.1, NC_WINDOW, NC_WINDOW);
diff --git a/source/blender/makesdna/DNA_ID.h b/source/blender/makesdna/DNA_ID.h
index 5c1bfc229da..feeb2d5e4d7 100644
--- a/source/blender/makesdna/DNA_ID.h
+++ b/source/blender/makesdna/DNA_ID.h
@@ -172,6 +172,13 @@ enum ePreviewImage_Flag {
 	PRV_USER_EDITED      = (1 << 1),  /* if user-edited, do not auto-update this anymore! */
 };
 
+/* for PreviewImage->tag */
+enum  {
+	PRV_TAG_DEFFERED           = (1 << 0),  /* Actual loading of preview is deffered. */
+	PRV_TAG_DEFFERED_RENDERING = (1 << 1),  /* Deffered preview is being loaded. */
+	PRV_TAG_DEFFERED_DELETE    = (1 << 2),  /* Deffered preview should be deleted asap. */
+};
+
 typedef struct PreviewImage {
 	/* All values of 2 are really NUM_ICON_SIZES */
 	unsigned int w[2];
@@ -184,12 +191,12 @@ typedef struct PreviewImage {
 	struct GPUTexture *gputexture[2];
 	int icon_id;  /* Used by previews outside of ID context. */
 
-	char pad[3];
-	char use_deferred;  /* for now a mere bool, if we add more deferred loading methods we can switch to bitflag. */
+	short tag;  /* Runtime data. */
+	char pad[2];
 } PreviewImage;
 
 #define PRV_DEFERRED_DATA(prv) \
-	(CHECK_TYPE_INLINE(prv, PreviewImage *), BLI_assert((prv)->use_deferred), (void *)((prv) + 1))
+	(CHECK_TYPE_INLINE(prv, PreviewImage *), BLI_assert((prv)->tag & PRV_TAG_DEFFERED), (void *)((prv) + 1))
 
 /**
  * Defines for working with IDs.