[Bf-blender-cvs] [665ac71] asset-experiments: Add API to IMB_thumb to enable thread safety.

Bastien Montagne noreply at git.blender.org
Thu Jul 2 21:22:23 CEST 2015


Commit: 665ac71aeee0f9ba34beefb006321ebeceed9da8
Author: Bastien Montagne
Date:   Thu Jul 2 13:31:38 2015 +0200
Branches: asset-experiments
https://developer.blender.org/rB665ac71aeee0f9ba34beefb006321ebeceed9da8

Add API to IMB_thumb to enable thread safety.

Indeed, though rather unlucky, we may end up handling same source file from
different preview threads, which could lead to conflicts and bugs.

So idea is to add a simple way of locking a given source file path, since
thumb handling of different paths shall never conflict.

Note that this adds some 'generic' stuff to GHash and Threads area, that are to
be committed separately of course.

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

M	source/blender/blenlib/BLI_ghash.h
M	source/blender/blenlib/BLI_threads.h
M	source/blender/blenlib/intern/BLI_ghash.c
M	source/blender/blenlib/intern/threads.c
M	source/blender/editors/space_file/filelist.c
M	source/blender/imbuf/IMB_thumbs.h
M	source/blender/imbuf/intern/thumbs.c

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

diff --git a/source/blender/blenlib/BLI_ghash.h b/source/blender/blenlib/BLI_ghash.h
index a9f8d9f..217f469 100644
--- a/source/blender/blenlib/BLI_ghash.h
+++ b/source/blender/blenlib/BLI_ghash.h
@@ -237,6 +237,9 @@ void   BLI_gset_clear(GSet *gs, GSetKeyFreeFP keyfreefp);
 GSet *BLI_gset_ptr_new_ex(const char *info,
                           const unsigned int nentries_reserve) ATTR_MALLOC ATTR_WARN_UNUSED_RESULT;
 GSet *BLI_gset_ptr_new(const char *info);
+GSet *BLI_gset_str_new_ex(const char *info,
+                          const unsigned int nentries_reserve) ATTR_MALLOC ATTR_WARN_UNUSED_RESULT;
+GSet *BLI_gset_str_new(const char *info);
 GSet *BLI_gset_pair_new_ex(const char *info,
                             const unsigned int nentries_reserve) ATTR_MALLOC ATTR_WARN_UNUSED_RESULT;
 GSet *BLI_gset_pair_new(const char *info) ATTR_MALLOC ATTR_WARN_UNUSED_RESULT;
diff --git a/source/blender/blenlib/BLI_threads.h b/source/blender/blenlib/BLI_threads.h
index b2ead15..6ae833d 100644
--- a/source/blender/blenlib/BLI_threads.h
+++ b/source/blender/blenlib/BLI_threads.h
@@ -158,6 +158,7 @@ typedef pthread_cond_t ThreadCondition;
 
 void BLI_condition_init(ThreadCondition *cond);
 void BLI_condition_wait(ThreadCondition *cond, ThreadMutex *mutex);
+void BLI_condition_wait_global_mutex(ThreadCondition *cond, const int type);
 void BLI_condition_notify_one(ThreadCondition *cond);
 void BLI_condition_notify_all(ThreadCondition *cond);
 void BLI_condition_end(ThreadCondition *cond);
diff --git a/source/blender/blenlib/intern/BLI_ghash.c b/source/blender/blenlib/intern/BLI_ghash.c
index 9287d62..91ac0ce 100644
--- a/source/blender/blenlib/intern/BLI_ghash.c
+++ b/source/blender/blenlib/intern/BLI_ghash.c
@@ -1356,6 +1356,15 @@ GSet *BLI_gset_ptr_new(const char *info)
 	return BLI_gset_ptr_new_ex(info, 0);
 }
 
+GSet *BLI_gset_str_new_ex(const char *info, const unsigned int nentries_reserve)
+{
+	return BLI_gset_new_ex(BLI_ghashutil_strhash_p, BLI_ghashutil_strcmp, info, nentries_reserve);
+}
+GSet *BLI_gset_str_new(const char *info)
+{
+	return BLI_gset_str_new_ex(info, 0);
+}
+
 GSet *BLI_gset_pair_new_ex(const char *info, const unsigned int nentries_reserve)
 {
 	return BLI_gset_new_ex(BLI_ghashutil_pairhash, BLI_ghashutil_paircmp, info, nentries_reserve);
diff --git a/source/blender/blenlib/intern/threads.c b/source/blender/blenlib/intern/threads.c
index 42f7a74..b609818 100644
--- a/source/blender/blenlib/intern/threads.c
+++ b/source/blender/blenlib/intern/threads.c
@@ -389,56 +389,45 @@ int BLI_system_num_threads_override_get(void)
 
 /* Global Mutex Locks */
 
+static ThreadMutex *global_mutex_from_type(const int type)
+{
+	switch (type) {
+		case LOCK_IMAGE:
+			return &_image_lock;
+		case LOCK_DRAW_IMAGE:
+			return &_image_draw_lock;
+		case LOCK_VIEWER:
+			return &_viewer_lock;
+		case LOCK_CUSTOM1:
+			return &_custom1_lock;
+		case LOCK_RCACHE:
+			return &_rcache_lock;
+		case LOCK_OPENGL:
+			return &_opengl_lock;
+		case LOCK_NODES:
+			return &_nodes_lock;
+		case LOCK_MOVIECLIP:
+			return &_movieclip_lock;
+		case LOCK_COLORMANAGE:
+			return &_colormanage_lock;
+		case LOCK_FFTW:
+			return &_fftw_lock;
+		case LOCK_VIEW3D:
+			return &_view3d_lock;
+		default:
+			BLI_assert(0);
+			return NULL;
+	}
+}
+
 void BLI_lock_thread(int type)
 {
-	if (type == LOCK_IMAGE)
-		pthread_mutex_lock(&_image_lock);
-	else if (type == LOCK_DRAW_IMAGE)
-		pthread_mutex_lock(&_image_draw_lock);
-	else if (type == LOCK_VIEWER)
-		pthread_mutex_lock(&_viewer_lock);
-	else if (type == LOCK_CUSTOM1)
-		pthread_mutex_lock(&_custom1_lock);
-	else if (type == LOCK_RCACHE)
-		pthread_mutex_lock(&_rcache_lock);
-	else if (type == LOCK_OPENGL)
-		pthread_mutex_lock(&_opengl_lock);
-	else if (type == LOCK_NODES)
-		pthread_mutex_lock(&_nodes_lock);
-	else if (type == LOCK_MOVIECLIP)
-		pthread_mutex_lock(&_movieclip_lock);
-	else if (type == LOCK_COLORMANAGE)
-		pthread_mutex_lock(&_colormanage_lock);
-	else if (type == LOCK_FFTW)
-		pthread_mutex_lock(&_fftw_lock);
-	else if (type == LOCK_VIEW3D)
-		pthread_mutex_lock(&_view3d_lock);
+	pthread_mutex_lock(global_mutex_from_type(type));
 }
 
 void BLI_unlock_thread(int type)
 {
-	if (type == LOCK_IMAGE)
-		pthread_mutex_unlock(&_image_lock);
-	else if (type == LOCK_DRAW_IMAGE)
-		pthread_mutex_unlock(&_image_draw_lock);
-	else if (type == LOCK_VIEWER)
-		pthread_mutex_unlock(&_viewer_lock);
-	else if (type == LOCK_CUSTOM1)
-		pthread_mutex_unlock(&_custom1_lock);
-	else if (type == LOCK_RCACHE)
-		pthread_mutex_unlock(&_rcache_lock);
-	else if (type == LOCK_OPENGL)
-		pthread_mutex_unlock(&_opengl_lock);
-	else if (type == LOCK_NODES)
-		pthread_mutex_unlock(&_nodes_lock);
-	else if (type == LOCK_MOVIECLIP)
-		pthread_mutex_unlock(&_movieclip_lock);
-	else if (type == LOCK_COLORMANAGE)
-		pthread_mutex_unlock(&_colormanage_lock);
-	else if (type == LOCK_FFTW)
-		pthread_mutex_unlock(&_fftw_lock);
-	else if (type == LOCK_VIEW3D)
-		pthread_mutex_unlock(&_view3d_lock);
+	pthread_mutex_unlock(global_mutex_from_type(type));
 }
 
 /* Mutex Locks */
@@ -619,6 +608,11 @@ void BLI_condition_wait(ThreadCondition *cond, ThreadMutex *mutex)
 	pthread_cond_wait(cond, mutex);
 }
 
+void BLI_condition_wait_global_mutex(ThreadCondition *cond, const int type)
+{
+	pthread_cond_wait(cond, global_mutex_from_type(type));
+}
+
 void BLI_condition_notify_one(ThreadCondition *cond)
 {
 	pthread_cond_signal(cond);
diff --git a/source/blender/editors/space_file/filelist.c b/source/blender/editors/space_file/filelist.c
index fd9a5ce..6d0ed2d 100644
--- a/source/blender/editors/space_file/filelist.c
+++ b/source/blender/editors/space_file/filelist.c
@@ -1079,7 +1079,11 @@ static void filelist_cache_previewf(TaskPool *pool, void *taskdata, int threadid
 		else if (preview->flags & FILE_TYPE_FTFONT) {
 			source = THB_SOURCE_FONT;
 		}
+
+		IMB_thumb_lock_path(preview->path);
 		preview->img = IMB_thumb_manage(preview->path, THB_LARGE, source);
+		IMB_thumb_unlock_path(preview->path);
+
 		BLI_thread_queue_push(cache->previews_done, preview);
 	}
 
@@ -1100,6 +1104,7 @@ static void filelist_cache_preview_ensure_running(FileListEntryCache *cache)
 		while (num_tasks--) {
 			BLI_task_pool_push(pool, filelist_cache_previewf, cache, false, TASK_PRIORITY_HIGH);
 		}
+		IMB_thumb_locks_acquire();
 	}
 	cache->previews_timestamp = 0.0;
 }
@@ -1141,6 +1146,8 @@ static void filelist_cache_previews_free(FileListEntryCache *cache, const bool s
 		cache->previews_pool = NULL;
 		cache->previews_todo = NULL;
 		cache->previews_done = NULL;
+
+		IMB_thumb_locks_release();
 	}
 	if (set_inactive) {
 		cache->flags &= ~FLC_PREVIEWS_ACTIVE;
diff --git a/source/blender/imbuf/IMB_thumbs.h b/source/blender/imbuf/IMB_thumbs.h
index 25cf7e8..9fce4f9 100644
--- a/source/blender/imbuf/IMB_thumbs.h
+++ b/source/blender/imbuf/IMB_thumbs.h
@@ -91,6 +91,12 @@ void   IMB_thumb_overlay_blend(unsigned int *thumb, int width, int height, float
 ImBuf *IMB_thumb_load_font(const char *filename, unsigned int x, unsigned int y);
 bool IMB_thumb_load_font_get_hash(char *r_hash);
 
+/* Threading */
+void IMB_thumb_locks_acquire(void);
+void IMB_thumb_locks_release(void);
+void IMB_thumb_lock_path(const char *path);
+void IMB_thumb_unlock_path(const char *path);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
diff --git a/source/blender/imbuf/intern/thumbs.c b/source/blender/imbuf/intern/thumbs.c
index 4c44da3..3e88c1c 100644
--- a/source/blender/imbuf/intern/thumbs.c
+++ b/source/blender/imbuf/intern/thumbs.c
@@ -32,12 +32,16 @@
 #include <stdio.h>
 #include <stdlib.h>
 
+#include "MEM_guardedalloc.h"
+
 #include "BLI_utildefines.h"
 #include "BLI_string.h"
 #include "BLI_path_util.h"
 #include "BLI_fileops.h"
+#include "BLI_ghash.h"
 #include "BLI_hash_md5.h"
 #include "BLI_system.h"
+#include "BLI_threads.h"
 #include BLI_SYSTEM_PID_H
 
 #include "BLO_readfile.h"  /* XXX Hope this is not badlevel! */
@@ -624,3 +628,77 @@ ImBuf *IMB_thumb_manage(const char *org_path, ThumbSize size, ThumbSource source
 
 	return img;
 }
+
+/* ***** Threading ***** */
+/* Thumbnail handling is not really threadsafe in itself.
+ * However, as long as we do not operate on the same file, we shall have no collision.
+ * So idea is to 'lock' a given source file path.
+ */
+
+static struct IMBThumbLocks {
+	GSet *locked_paths;
+	int lock_counter;
+	ThreadCondition cond;
+} thumb_locks = {0};
+
+void IMB_thumb_locks_acquire(void) {
+	BLI_lock_thread(LOCK_IMAGE);
+
+	if (thumb_locks.lock_counter == 0) {
+		BLI_assert(thumb_locks.locked_paths == NULL);
+		thumb_locks.locked_paths = BLI_gset_str_new(__func__);
+		BLI_condition_init(&thumb_locks.cond);
+	}
+	thumb_locks.lock_counter++;
+
+	BLI_assert(thumb_locks.locked_paths != NULL);
+	BLI_assert(thumb_locks.lock_counter > 0);
+	BLI_unlock_thread(LOCK_IMAGE);
+}
+
+void IMB_thumb_locks_release(void) {
+	BLI_lock_thread(LOCK_IMAGE);
+	BLI_assert((thumb_locks.locked_paths != NULL) && (thumb_locks.lock_counter > 0));
+
+	thumb_locks.lock_counter--;
+	if (thumb_locks.lock_counter == 0) {
+		BLI_gset_free(thumb_locks.locked_paths, MEM_freeN);
+		thumb_locks.locked_paths = NULL;
+		BLI_condition_end(&thumb_locks.cond);
+	}
+
+	BLI_unlock_thread(LOCK_IMAGE);
+}
+
+void IMB_thumb_lock_path(const char *path) {
+	void *key = BLI_strdup(path);
+
+	BLI_lock_thread(LOCK_IMAGE);
+	BLI_assert((thumb_locks.locked_paths != NULL) && (thumb_locks.lock_counter > 0));
+
+	if (thumb_locks.locked_paths) {
+		while (!BLI_gset_add(thumb_locks.locked_paths, key)) {
+			BLI_condition_wait_global_mutex(&thumb_locks.cond, LOCK_IMAGE);
+		}
+//		printf("%s: locked %s\n", __func__, path);
+	}
+
+	BLI_unlock_thread(LOCK_IMAGE);
+}
+
+void IMB_thumb_unlock_path(const char *path) {
+	const void *key = path;
+
+	BLI_lock_thread(LOCK_IMAGE);
+	BLI_assert((thumb_locks.locked_paths != NULL) && (thumb_locks.lock_counter > 0));
+
+	if (thumb_locks.locked_paths) {
+		if (!BLI_gset_remove(thumb_locks.locked_paths, key, MEM_freeN)) {
+			BLI_assert(0);
+		}
+		BLI_condition_notify_all(&thumb_locks.cond);
+//		printf("%s: UN-locked %s\n", __func__, path);
+	}
+
+	BLI_unlock_thread(LOCK_IMAGE);
+}




More information about the Bf-blender-cvs mailing list