Fix non-thread-safe access to multilayer image in Compositor

Conversion of compositor node tree to operation is done in a job thread,
and the main thread might modify the image data-block at the same time.

This change fixes it by making it so compositor uses acquire/release
semantic for the image data-block, and making it so the image locks its
render result, preventing other threads from modifying it.

Ref #121761

Pull Request: https://projects.blender.org/blender/blender/pulls/122105

source/blender/blenkernel/BKE_image.h

@@ -298,8 +298,24 @@ void BKE_image_multiview_index(const struct Image *ima, struct ImageUser *iuser)
 bool BKE_image_is_multilayer(const struct Image *ima);
 bool BKE_image_is_multiview(const struct Image *ima);
 bool BKE_image_is_stereo(const struct Image *ima);
-struct RenderResult *BKE_image_acquire_renderresult(struct Scene *scene, struct Image *ima);
-void BKE_image_release_renderresult(struct Scene *scene, struct Image *ima);
+
+/**
+ * Acquire render result associated with the give image.
+ *
+ * If the result is non a null pointer  then the image is locking its render result part of
+ * operations, preventing other threads from modifying the result. It is then required to call
+ * #BKE_image_release_renderresult with the render result returned by this function.
+ *
+ * It is allowed to call #BKE_image_release_renderresult with render_result of nullptr.
+ *
+ * NOTE: It is not possible to use functions like #BKE_image_acquire_ibuf and
+ * #BKE_image_acquire_multilayer_view_ibuf as the same internal mutex as the image cache is used
+ * to protect the render result. When RenderResult is acquired, access image buffers directly from
+ * its passes, with user-increment when a more permanent reference is needed instead of the image
+ * buffer acquire/release functions.
+ */
+RenderResult *BKE_image_acquire_renderresult(Scene *scene, Image *ima);
+void BKE_image_release_renderresult(Scene *scene, Image *ima, RenderResult *render_result);
 
 /**
  * For multi-layer images as well as for single-layer.

source/blender/blenkernel/intern/image.cc

@@ -3822,11 +3822,14 @@ static void image_init_multilayer_multiview(Image *ima, RenderResult *rr)
 
 RenderResult *BKE_image_acquire_renderresult(Scene *scene, Image *ima)
 {
+  BLI_mutex_lock(static_cast<ThreadMutex *>(ima->runtime.cache_mutex));
+
   RenderResult *rr = nullptr;
+
   if (ima->rr) {
     rr = ima->rr;
   }
-  else if (ima->type == IMA_TYPE_R_RESULT) {
+  else if (scene && ima->type == IMA_TYPE_R_RESULT) {
     if (ima->render_slot == ima->last_render_slot) {
       rr = RE_AcquireResultRead(RE_GetSceneRender(scene));
     }
@@ -3839,19 +3842,27 @@ RenderResult *BKE_image_acquire_renderresult(Scene *scene, Image *ima)
     image_init_multilayer_multiview(ima, rr);
   }
 
+  if (!rr) {
+    BLI_mutex_unlock(static_cast<ThreadMutex *>(ima->runtime.cache_mutex));
+  }
+
   return rr;
 }
 
-void BKE_image_release_renderresult(Scene *scene, Image *ima)
+void BKE_image_release_renderresult(Scene *scene, Image *ima, RenderResult *render_result)
 {
   if (ima->rr) {
     /* pass */
   }
-  else if (ima->type == IMA_TYPE_R_RESULT) {
+  else if (scene && ima->type == IMA_TYPE_R_RESULT) {
     if (ima->render_slot == ima->last_render_slot) {
       RE_ReleaseResult(RE_GetSceneRender(scene));
     }
   }
+
+  if (render_result) {
+    BLI_mutex_unlock(static_cast<ThreadMutex *>(ima->runtime.cache_mutex));
+  }
 }
 
 bool BKE_image_is_openexr(Image *ima)

source/blender/blenkernel/intern/image_save.cc

@@ -414,7 +414,7 @@ static bool image_save_single(ReportList *reports,
                     STEREO_LEFT_NAME,
                     STEREO_RIGHT_NAME);
         BKE_image_release_ibuf(ima, ibuf, lock);
-        BKE_image_release_renderresult(opts->scene, ima);
+        BKE_image_release_renderresult(opts->scene, ima, rr);
         return ok;
       }
 
@@ -428,7 +428,7 @@ static bool image_save_single(ReportList *reports,
                     STEREO_LEFT_NAME,
                     STEREO_RIGHT_NAME);
         BKE_image_release_ibuf(ima, ibuf, lock);
-        BKE_image_release_renderresult(opts->scene, ima);
+        BKE_image_release_renderresult(opts->scene, ima, rr);
         return ok;
       }
     }
@@ -606,7 +606,7 @@ static bool image_save_single(ReportList *reports,
   }
 
   if (rr) {
-    BKE_image_release_renderresult(opts->scene, ima);
+    BKE_image_release_renderresult(opts->scene, ima, rr);
   }
 
   return ok;

source/blender/compositor/nodes/COM_ImageNode.cc

@@ -72,8 +72,9 @@ void ImageNode::convert_to_operations(NodeConverter &converter,
   if (image && image->type == IMA_TYPE_MULTILAYER) {
     bool is_multilayer_ok = false;
     ImBuf *ibuf = BKE_image_acquire_ibuf(image, imageuser, nullptr);
-    if (image->rr) {
-      RenderLayer *rl = (RenderLayer *)BLI_findlink(&image->rr->layers, imageuser->layer);
+    RenderResult *rr = BKE_image_acquire_renderresult(nullptr, image);
+    if (rr) {
+      RenderLayer *rl = (RenderLayer *)BLI_findlink(&rr->layers, imageuser->layer);
       if (rl) {
         is_multilayer_ok = true;
 
@@ -165,6 +166,7 @@ void ImageNode::convert_to_operations(NodeConverter &converter,
         }
       }
     }
+    BKE_image_release_renderresult(nullptr, image, rr);
     BKE_image_release_ibuf(image, ibuf, nullptr);
 
     /* without this, multilayer that fail to load will crash blender #32490. */

source/blender/editors/space_image/image_buttons.cc

@@ -211,7 +211,7 @@ static void ui_imageuser_layer_menu(bContext * /*C*/, uiLayout *layout, void *rn
            0.0,
            "");
 
-  BKE_image_release_renderresult(scene, image);
+  BKE_image_release_renderresult(scene, image, rr);
 }
 
 static void ui_imageuser_pass_menu(bContext * /*C*/, uiLayout *layout, void *rnd_pt)
@@ -285,7 +285,7 @@ static void ui_imageuser_pass_menu(bContext * /*C*/, uiLayout *layout, void *rnd
 
   BLI_freelistN(&added_passes);
 
-  BKE_image_release_renderresult(scene, image);
+  BKE_image_release_renderresult(scene, image, rr);
 }
 
 /**************************** view menus *****************************/
@@ -342,7 +342,7 @@ static void ui_imageuser_view_menu_rr(bContext * /*C*/, uiLayout *layout, void *
               "");
   }
 
-  BKE_image_release_renderresult(scene, image);
+  BKE_image_release_renderresult(scene, image, rr);
 }
 
 static void ui_imageuser_view_menu_multiview(bContext * /*C*/, uiLayout *layout, void *rnd_pt)
@@ -435,7 +435,7 @@ static bool ui_imageuser_layer_menu_step(bContext *C, int direction, void *rnd_p
     BLI_assert(0);
   }
 
-  BKE_image_release_renderresult(scene, image);
+  BKE_image_release_renderresult(scene, image, rr);
 
   if (changed) {
     BKE_image_multilayer_index(rr, iuser);
@@ -459,7 +459,7 @@ static bool ui_imageuser_pass_menu_step(bContext *C, int direction, void *rnd_pt
 
   rr = BKE_image_acquire_renderresult(scene, image);
   if (UNLIKELY(rr == nullptr)) {
-    BKE_image_release_renderresult(scene, image);
+    BKE_image_release_renderresult(scene, image, rr);
     return false;
   }
 
@@ -469,13 +469,13 @@ static bool ui_imageuser_pass_menu_step(bContext *C, int direction, void *rnd_pt
 
   rl = static_cast<RenderLayer *>(BLI_findlink(&rr->layers, layer));
   if (rl == nullptr) {
-    BKE_image_release_renderresult(scene, image);
+    BKE_image_release_renderresult(scene, image, rr);
     return false;
   }
 
   rpass = static_cast<RenderPass *>(BLI_findlink(&rl->passes, iuser->pass));
   if (rpass == nullptr) {
-    BKE_image_release_renderresult(scene, image);
+    BKE_image_release_renderresult(scene, image, rr);
     return false;
   }
 
@@ -497,7 +497,7 @@ static bool ui_imageuser_pass_menu_step(bContext *C, int direction, void *rnd_pt
     int rp_index = 0;
 
     if (iuser->pass == 0) {
-      BKE_image_release_renderresult(scene, image);
+      BKE_image_release_renderresult(scene, image, rr);
       return false;
     }
 
@@ -513,7 +513,7 @@ static bool ui_imageuser_pass_menu_step(bContext *C, int direction, void *rnd_pt
     BLI_assert(0);
   }
 
-  BKE_image_release_renderresult(scene, image);
+  BKE_image_release_renderresult(scene, image, rr);
 
   if (changed) {
     BKE_image_multilayer_index(rr, iuser);
@@ -780,7 +780,7 @@ void uiTemplateImage(uiLayout *layout,
       /* Use #BKE_image_acquire_renderresult so we get the correct slot in the menu. */
       rr = BKE_image_acquire_renderresult(scene, ima);
       uiblock_layer_pass_buttons(layout, ima, rr, iuser, menus_width, &ima->render_slot);
-      BKE_image_release_renderresult(scene, ima);
+      BKE_image_release_renderresult(scene, ima, rr);
     }
 
     return;
@@ -1153,7 +1153,7 @@ void uiTemplateImageLayers(uiLayout *layout, bContext *C, Image *ima, ImageUser
     rr = BKE_image_acquire_renderresult(scene, ima);
     uiblock_layer_pass_buttons(
         layout, ima, rr, iuser, menus_width, is_render_result ? &ima->render_slot : nullptr);
-    BKE_image_release_renderresult(scene, ima);
+    BKE_image_release_renderresult(scene, ima, rr);
   }
 }
 

source/blender/editors/space_image/image_draw.cc

@@ -79,7 +79,7 @@ static void draw_render_info(
     ED_region_info_draw(region, rr->text, fill_color, true);
   }
 
-  BKE_image_release_renderresult(stats_scene, ima);
+  BKE_image_release_renderresult(stats_scene, ima, rr);
 
   if (re) {
     int total_tiles;