Fix #32087: Crash while changing values in comp editor (bt and blender included)
authorSergey Sharybin <sergey.vfx@gmail.com>
Fri, 13 Jul 2012 13:47:13 +0000 (13:47 +0000)
committerSergey Sharybin <sergey.vfx@gmail.com>
Fri, 13 Jul 2012 13:47:13 +0000 (13:47 +0000)
Issue was caused by threading conflict between compositor output node which
is freeing buffers used by render result image and image draw code which
could use buffers at the same time as compositor frees this buffers.

Solved by adding adding  lock around viewer image invalidation and image
drawing.

Use renamed LOCK_PREVIEW mutex for this, which si not called LOCK_DRAW_IMAGE.
With new compositor locking for preview is not needed so it could be removed.

Added the same lock around viewer operation which also frees buffers used
by viewer image. It's actually quite difficult to check whether this is
indeed needed. This code seems to be using acquire/release technique, but
somehow acquiring ImBuf before invalidating it in compositor operation
doesn't resolve the issue, so probably it's not actually locking acquire
and things should be checked deeper.

source/blender/blenlib/BLI_threads.h
source/blender/blenlib/intern/threads.c
source/blender/compositor/operations/COM_CompositorOperation.cpp
source/blender/compositor/operations/COM_ViewerBaseOperation.cpp
source/blender/editors/space_image/image_draw.c
source/blender/editors/space_node/node_draw.c
source/blender/nodes/composite/node_composite_util.c

index 3b75797a3a9584f5e850694b1e77214f53cf0513..902373bcd6bd06f85de82e9dbe872a6a71187b01 100644 (file)
@@ -70,7 +70,7 @@ int     BLI_system_thread_count(void); /* gets the number of threads the system
  * One custom lock available now. can be extended. */
 
 #define LOCK_IMAGE      0
-#define LOCK_PREVIEW    1
+#define LOCK_DRAW_IMAGE 1
 #define LOCK_VIEWER     2
 #define LOCK_CUSTOM1    3
 #define LOCK_RCACHE     4
index 348fa29eae79be6db99679e965b53b7dd106b59d..9994f89acd5a154844ac7e31badfe8048b00b4ed 100644 (file)
@@ -106,7 +106,7 @@ static void *thread_tls_data;
  ************************************************ */
 static pthread_mutex_t _malloc_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _image_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_mutex_t _preview_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_mutex_t _image_draw_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _viewer_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _custom1_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _rcache_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -337,8 +337,8 @@ void BLI_lock_thread(int type)
 {
        if (type == LOCK_IMAGE)
                pthread_mutex_lock(&_image_lock);
-       else if (type == LOCK_PREVIEW)
-               pthread_mutex_lock(&_preview_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)
@@ -357,8 +357,8 @@ void BLI_unlock_thread(int type)
 {
        if (type == LOCK_IMAGE)
                pthread_mutex_unlock(&_image_lock);
-       else if (type == LOCK_PREVIEW)
-               pthread_mutex_unlock(&_preview_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)
index 445b0634ced485ae65f76c31d8cecd003df2d71b..43aad4f19d972a02c23fea8122d95ce272927741 100644 (file)
@@ -27,6 +27,7 @@
 #include "BKE_image.h"
 
 extern "C" {
+       #include "BLI_threads.h"
        #include "RE_pipeline.h"
        #include "RE_shader_ext.h"
        #include "RE_render_ext.h"
@@ -63,6 +64,7 @@ void CompositorOperation::deinitExecution()
                const RenderData *rd = this->m_rd;
                Render *re = RE_GetRender_FromData(rd);
                RenderResult *rr = RE_AcquireResultWrite(re);
+
                if (rr) {
                        if (rr->rectf != NULL) {
                                MEM_freeN(rr->rectf);
@@ -75,7 +77,9 @@ void CompositorOperation::deinitExecution()
                        }
                }
 
+               BLI_lock_thread(LOCK_DRAW_IMAGE);
                BKE_image_signal(BKE_image_verify_viewer(IMA_TYPE_R_RESULT, "Render Result"), NULL, IMA_SIGNAL_FREE);
+               BLI_unlock_thread(LOCK_DRAW_IMAGE);
 
                if (re) {
                        RE_ReleaseResult(re);
index 446b169763c380e038d59abe0677e51df9c566bc..2470b239987546209f1aa29c47f63b2c6e5f357b 100644 (file)
@@ -62,6 +62,8 @@ void ViewerBaseOperation::initImage()
        
        if (!ibuf) return;
        if (ibuf->x != (int)getWidth() || ibuf->y != (int)getHeight()) {
+               BLI_lock_thread(LOCK_DRAW_IMAGE);
+
                imb_freerectImBuf(ibuf);
                imb_freerectfloatImBuf(ibuf);
                IMB_freezbuffloatImBuf(ibuf);
@@ -70,6 +72,8 @@ void ViewerBaseOperation::initImage()
                imb_addrectImBuf(ibuf);
                imb_addrectfloatImBuf(ibuf);
                anImage->ok = IMA_OK_LOADED;
+
+               BLI_unlock_thread(LOCK_DRAW_IMAGE);
        }
        
        /* now we combine the input with ibuf */
index ac71eb972f64788530c753f12ccd7c7b1067815b..88eb280ea6bc0f9dbb78b9bbc7fc76404dfeaeec 100644 (file)
@@ -728,11 +728,21 @@ void draw_image_main(SpaceImage *sima, ARegion *ar, Scene *scene)
        /* retrieve the image and information about it */
        ima = ED_space_image(sima);
        ED_space_image_zoom(sima, ar, &zoomx, &zoomy);
-       ibuf = ED_space_image_acquire_buffer(sima, &lock);
 
        show_viewer = (ima && ima->source == IMA_SRC_VIEWER);
        show_render = (show_viewer && ima->type == IMA_TYPE_R_RESULT);
 
+       if (show_viewer) {
+               /* use locked draw for drawing viewer image buffer since the conpositor
+                * is running in separated thread and compositor could free this buffers.
+                * other images are not modifying in such a way so they does not require
+                * lock (sergey)
+                */
+               BLI_lock_thread(LOCK_DRAW_IMAGE);
+       }
+
+       ibuf = ED_space_image_acquire_buffer(sima, &lock);
+
        /* draw the image or grid */
        if (ibuf == NULL)
                ED_region_grid_draw(ar, zoomx, zoomy);
@@ -770,5 +780,8 @@ void draw_image_main(SpaceImage *sima, ARegion *ar, Scene *scene)
        /* render info */
        if (ima && show_render)
                draw_render_info(scene, ima, ar);
-}
 
+       if (show_viewer) {
+               BLI_unlock_thread(LOCK_DRAW_IMAGE);
+       }
+}
index 17545ddc4de3c99147c719635235794923e47e0b..adf523133071c60191d5ce93294b5a2731bca067 100644 (file)
@@ -335,9 +335,6 @@ static void node_update_basis(const bContext *C, bNodeTree *ntree, bNode *node)
 
        /* preview rect? */
        if (node->flag & NODE_PREVIEW) {
-               /* only recalculate size when there's a preview actually, otherwise we use stored result */
-               BLI_lock_thread(LOCK_PREVIEW);
-
                if (node->preview && node->preview->rect) {
                        float aspect = 1.0f;
                        
@@ -374,8 +371,6 @@ static void node_update_basis(const bContext *C, bNodeTree *ntree, bNode *node)
                        node->prvr.ymin = dy - oldh;
                        dy = node->prvr.ymin - NODE_DYS / 2;
                }
-
-               BLI_unlock_thread(LOCK_PREVIEW);
        }
 
        /* buttons rect? */
@@ -861,10 +856,8 @@ static void node_draw_basis(const bContext *C, ARegion *ar, SpaceNode *snode, bN
        
        /* preview */
        if (node->flag & NODE_PREVIEW) {
-               BLI_lock_thread(LOCK_PREVIEW);
                if (node->preview && node->preview->rect && !BLI_rctf_is_empty(&node->prvr))
                        node_draw_preview(node->preview, &node->prvr);
-               BLI_unlock_thread(LOCK_PREVIEW);
        }
        
        UI_ThemeClearColor(color_id);
index 98ded3ea3b356d7afd90665cdb65adaa05e4ed39..1b9ff610e4c0f471fd0283d5cd51a242959f4555 100644 (file)
@@ -647,7 +647,7 @@ void generate_preview(void *data, bNode *node, CompBuf *stackbuf)
                if (stackbuf_use!=stackbuf)
                        free_compbuf(stackbuf_use);
 
-               BLI_lock_thread(LOCK_PREVIEW);
+               // BLI_lock_thread(LOCK_PREVIEW);
 
                if (preview->rect)
                        MEM_freeN(preview->rect);
@@ -655,7 +655,7 @@ void generate_preview(void *data, bNode *node, CompBuf *stackbuf)
                preview->ysize= ysize;
                preview->rect= rect;
 
-               BLI_unlock_thread(LOCK_PREVIEW);
+               // BLI_unlock_thread(LOCK_PREVIEW);
        }
 }