Commit: 8c4ddd5dde3a5f5ae0375c71e379e0fac287e3e6 Author: Sergey Sharybin Date: Mon Mar 14 11:32:46 2022 +0100 Branches: master https://developer.blender.org/rB8c4ddd5dde3a5f5ae0375c71e379e0fac287e3e6
Fix dead-lock in movie cache Steps to reproduce: - Add image sequence to movie clip editor. - Set cache limit to a low value in the user preferences. - Playback until old frames starts to be removed from cache. - Jump to the beginning of the image sequence. The reason of dead-lock comes from two factors: - Due to global nature of the cache limiter calls needs to be guarded with locks. - Image buffers stored in the cache can have their own cache (which is used for color management). Didn't find a better solution than to use recursive lock. Kind of makes sense since the thread-guardable resource is recursive (moviecache can have nested moviecaches). Differential Revision: https://developer.blender.org/D14331 =================================================================== M source/blender/imbuf/intern/moviecache.cc =================================================================== diff --git a/source/blender/imbuf/intern/moviecache.cc b/source/blender/imbuf/intern/moviecache.cc index a02a48522af..577f6562b9e 100644 --- a/source/blender/imbuf/intern/moviecache.cc +++ b/source/blender/imbuf/intern/moviecache.cc @@ -8,6 +8,7 @@ #undef DEBUG_MESSAGES #include <memory.h> +#include <mutex> #include <stdlib.h> /* for qsort */ #include "MEM_CacheLimiterC-Api.h" @@ -35,7 +36,12 @@ #endif static MEM_CacheLimiterC *limitor = NULL; -static pthread_mutex_t limitor_lock = BLI_MUTEX_INITIALIZER; + +/* Image buffers managed by a moviecache might be using their own movie caches (used by color + * management). In practice this means that, for example, freeing MovieCache used by MovieClip + * will request freeing MovieCache owned by ImBuf. Freeing MovieCache needs to be thread-safe, + * so regular mutex will not work here, hence the recursive lock. */ +static std::recursive_mutex limitor_lock; typedef struct MovieCache { char name[64]; @@ -107,9 +113,9 @@ static void moviecache_valfree(void *val) PRINT("%s: cache '%s' free item %p buffer %p\n", __func__, cache->name, item, item->ibuf); if (item->c_handle) { - BLI_mutex_lock(&limitor_lock); + limitor_lock.lock(); MEM_CacheLimiter_unmanage(item->c_handle); - BLI_mutex_unlock(&limitor_lock); + limitor_lock.unlock(); } if (item->ibuf) { @@ -339,7 +345,7 @@ static void do_moviecache_put(MovieCache *cache, void *userkey, ImBuf *ibuf, boo } if (need_lock) { - BLI_mutex_lock(&limitor_lock); + limitor_lock.lock(); } item->c_handle = MEM_CacheLimiter_insert(limitor, item); @@ -349,7 +355,7 @@ static void do_moviecache_put(MovieCache *cache, void *userkey, ImBuf *ibuf, boo MEM_CacheLimiter_unref(item->c_handle); if (need_lock) { - BLI_mutex_unlock(&limitor_lock); + limitor_lock.unlock(); } /* cache limiter can't remove unused keys which points to destroyed values */ @@ -371,7 +377,7 @@ bool IMB_moviecache_put_if_possible(MovieCache *cache, void *userkey, ImBuf *ibu elem_size = (ibuf == NULL) ? 0 : get_size_in_memory(ibuf); mem_limit = MEM_CacheLimiter_get_maximum(); - BLI_mutex_lock(&limitor_lock); + limitor_lock.lock(); mem_in_use = MEM_CacheLimiter_get_memory_in_use(limitor); if (mem_in_use + elem_size <= mem_limit) { @@ -379,7 +385,7 @@ bool IMB_moviecache_put_if_possible(MovieCache *cache, void *userkey, ImBuf *ibu result = true; } - BLI_mutex_unlock(&limitor_lock); + limitor_lock.unlock(); return result; } @@ -407,9 +413,9 @@ ImBuf *IMB_moviecache_get(MovieCache *cache, void *userkey, bool *r_is_cached_em if (item) { if (item->ibuf) { - BLI_mutex_lock(&limitor_lock); + limitor_lock.lock(); MEM_CacheLimiter_touch(item->c_handle); - BLI_mutex_unlock(&limitor_lock); + limitor_lock.unlock(); IMB_refImBuf(item->ibuf); _______________________________________________ Bf-blender-cvs mailing list Bf-blender-cvs@blender.org List details, subscription details or unsubscribe: https://lists.blender.org/mailman/listinfo/bf-blender-cvs
