diff options
author | Sergey Sharybin <sergey@blender.org> | 2022-03-14 13:32:46 +0300 |
---|---|---|
committer | Sergey Sharybin <sergey@blender.org> | 2022-03-14 16:46:49 +0300 |
commit | 8c4ddd5dde3a5f5ae0375c71e379e0fac287e3e6 (patch) | |
tree | 202b4c9fe743c53307d96835b69ac9065f8a4846 /source/blender/imbuf | |
parent | e0241e08609aebd2ba14075a046ce3e139096ce3 (diff) |
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
Diffstat (limited to 'source/blender/imbuf')
-rw-r--r-- | source/blender/imbuf/intern/moviecache.cc | 24 |
1 files changed, 15 insertions, 9 deletions
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); |