BLI: use explicit task isolation, no longer part of parallel operations

After looking into task isolation issues with Sergey, we couldn't find the
reason behind the deadlocks that we are getting in T87938 and a Sprite Fright
file involving motion blur renders.

There is no apparent place where we adding or waiting on tasks in a task group
from different isolation regions, which is what is known to cause problems. Yet
it still hangs. Either we do not understand some limitation of TBB isolation,
or there is a bug in TBB, but we could not figure it out.

Instead the idea is to use isolation only where we know we need it: when
holding a mutex lock and then doing some multithreaded operation within that
locked region. Three places where we do this now:
* Generated images
* Cached BVH tree building
* OpenVDB lazy grid loading

Compared to the more automatic approach previously used, there is the downside
that it is easy to miss places where we need isolation. Yet doing it more
automatically is also causing unexpected issue and bugs that we found no
solution for, so this seems better.

Patch implemented by Sergey and me.

Differential Revision: https://developer.blender.org/D11603

10 files changed, 102 insertions, 38 deletions

diff --git a/source/blender/blenkernel/intern/bvhutils.c b/source/blender/blenkernel/intern/bvhutils.c
index bc63e423c09..116e6279657 100644
--- a/source/blender/blenkernel/intern/bvhutils.c
+++ b/source/blender/blenkernel/intern/bvhutils.c
@@ -31,6 +31,7 @@
 
 #include "BLI_linklist.h"
 #include "BLI_math.h"
+#include "BLI_task.h"
 #include "BLI_threads.h"
 #include "BLI_utildefines.h"
 
@@ -160,6 +161,26 @@ void bvhcache_free(BVHCache *bvh_cache)
   MEM_freeN(bvh_cache);
 }
 
+/* BVH tree balancing inside a mutex lock must be run in isolation. Balancing
+ * is multithreaded, and we do not want the current thread to start another task
+ * that may involve acquiring the same mutex lock that it is waiting for. */
+static void bvhtree_balance_isolated(void *userdata)
+{
+  BLI_bvhtree_balance((BVHTree *)userdata);
+}
+
+static void bvhtree_balance(BVHTree *tree, const bool isolate)
+{
+  if (tree) {
+    if (isolate) {
+      BLI_task_isolate(bvhtree_balance_isolated, tree);
+    }
+    else {
+      BLI_bvhtree_balance(tree);
+    }
+  }
+}
+
 /** \} */
 /* -------------------------------------------------------------------- */
 /** \name Local Callbacks
@@ -566,7 +587,6 @@ static BVHTree *bvhtree_from_editmesh_verts_create_tree(float epsilon,
       BLI_bvhtree_insert(tree, i, eve->co, 1);
     }
     BLI_assert(BLI_bvhtree_get_len(tree) == verts_num_active);
-    BLI_bvhtree_balance(tree);
   }
 
   return tree;
@@ -600,7 +620,6 @@ static BVHTree *bvhtree_from_mesh_verts_create_tree(float epsilon,
         BLI_bvhtree_insert(tree, i, vert[i].co, 1);
       }
       BLI_assert(BLI_bvhtree_get_len(tree) == verts_num_active);
-      BLI_bvhtree_balance(tree);
     }
   }
 
@@ -649,6 +668,7 @@ BVHTree *bvhtree_from_editmesh_verts_ex(BVHTreeFromEditMesh *data,
     if (data->cached == false) {
       tree = bvhtree_from_editmesh_verts_create_tree(
           epsilon, tree_type, axis, em, verts_mask, verts_num_active);
+      bvhtree_balance(tree, true);
 
       /* Save on cache for later use */
       /* printf("BVHTree built and saved on cache\n"); */
@@ -660,6 +680,7 @@ BVHTree *bvhtree_from_editmesh_verts_ex(BVHTreeFromEditMesh *data,
   else {
     tree = bvhtree_from_editmesh_verts_create_tree(
         epsilon, tree_type, axis, em, verts_mask, verts_num_active);
+    bvhtree_balance(tree, false);
   }
 
   if (tree) {
@@ -711,6 +732,7 @@ BVHTree *bvhtree_from_mesh_verts_ex(BVHTreeFromMesh *data,
   if (in_cache == false) {
     tree = bvhtree_from_mesh_verts_create_tree(
         epsilon, tree_type, axis, vert, verts_num, verts_mask, verts_num_active);
+    bvhtree_balance(tree, bvh_cache_p != NULL);
 
     if (bvh_cache_p) {
       /* Save on cache for later use */
@@ -771,7 +793,6 @@ static BVHTree *bvhtree_from_editmesh_edges_create_tree(float epsilon,
       BLI_bvhtree_insert(tree, i, co[0], 2);
     }
     BLI_assert(BLI_bvhtree_get_len(tree) == edges_num_active);
-    BLI_bvhtree_balance(tree);
   }
 
   return tree;
@@ -809,7 +830,6 @@ static BVHTree *bvhtree_from_mesh_edges_create_tree(const MVert *vert,
 
         BLI_bvhtree_insert(tree, i, co[0], 2);
       }
-      BLI_bvhtree_balance(tree);
     }
   }
 
@@ -861,7 +881,7 @@ BVHTree *bvhtree_from_editmesh_edges_ex(BVHTreeFromEditMesh *data,
     if (data->cached == false) {
       tree = bvhtree_from_editmesh_edges_create_tree(
           epsilon, tree_type, axis, em, edges_mask, edges_num_active);
-
+      bvhtree_balance(tree, true);
       /* Save on cache for later use */
       /* printf("BVHTree built and saved on cache\n"); */
       bvhcache_insert(bvh_cache, tree, bvh_cache_type);
@@ -872,6 +892,7 @@ BVHTree *bvhtree_from_editmesh_edges_ex(BVHTreeFromEditMesh *data,
   else {
     tree = bvhtree_from_editmesh_edges_create_tree(
         epsilon, tree_type, axis, em, edges_mask, edges_num_active);
+    bvhtree_balance(tree, false);
   }
 
   if (tree) {
@@ -928,12 +949,17 @@ BVHTree *bvhtree_from_mesh_edges_ex(BVHTreeFromMesh *data,
         vert, edge, edges_num, edges_mask, edges_num_active, epsilon, tree_type, axis);
 
     if (bvh_cache_p) {
+      bvhtree_balance(tree, true);
+
       BVHCache *bvh_cache = *bvh_cache_p;
       /* Save on cache for later use */
       /* printf("BVHTree built and saved on cache\n"); */
       bvhcache_insert(bvh_cache, tree, bvh_cache_type);
       in_cache = true;
     }
+    else {
+      bvhtree_balance(tree, false);
+    }
   }
 
   if (bvh_cache_p) {
@@ -994,7 +1020,6 @@ static BVHTree *bvhtree_from_mesh_faces_create_tree(float epsilon,
         }
       }
       BLI_assert(BLI_bvhtree_get_len(tree) == faces_num_active);
-      BLI_bvhtree_balance(tree);
     }
   }
 
@@ -1057,6 +1082,7 @@ BVHTree *bvhtree_from_mesh_faces_ex(BVHTreeFromMesh *data,
   if (in_cache == false) {
     tree = bvhtree_from_mesh_faces_create_tree(
         epsilon, tree_type, axis, vert, face, numFaces, faces_mask, faces_num_active);
+    bvhtree_balance(tree, bvh_cache_p != NULL);
 
     if (bvh_cache_p) {
       /* Save on cache for later use */
@@ -1127,7 +1153,6 @@ static BVHTree *bvhtree_from_editmesh_looptri_create_tree(float epsilon,
         }
       }
       BLI_assert(BLI_bvhtree_get_len(tree) == looptri_num_active);
-      BLI_bvhtree_balance(tree);
     }
   }
 
@@ -1173,7 +1198,6 @@ static BVHTree *bvhtree_from_mesh_looptri_create_tree(float epsilon,
         }
       }
       BLI_assert(BLI_bvhtree_get_len(tree) == looptri_num_active);
-      BLI_bvhtree_balance(tree);
     }
   }
 
@@ -1229,6 +1253,7 @@ BVHTree *bvhtree_from_editmesh_looptri_ex(BVHTreeFromEditMesh *data,
     bool in_cache = bvhcache_find(
         bvh_cache_p, bvh_cache_type, &tree, &lock_started, mesh_eval_mutex);
     BVHCache *bvh_cache = *bvh_cache_p;
+    bvhtree_balance(tree, true);
 
     if (in_cache == false) {
       tree = bvhtree_from_editmesh_looptri_create_tree(
@@ -1243,6 +1268,7 @@ BVHTree *bvhtree_from_editmesh_looptri_ex(BVHTreeFromEditMesh *data,
   else {
     tree = bvhtree_from_editmesh_looptri_create_tree(
         epsilon, tree_type, axis, em, looptri_mask, looptri_num_active);
+    bvhtree_balance(tree, false);
   }
 
   if (tree) {
@@ -1303,6 +1329,8 @@ BVHTree *bvhtree_from_mesh_looptri_ex(BVHTreeFromMesh *data,
                                                  looptri_mask,
                                                  looptri_num_active);
 
+    bvhtree_balance(tree, bvh_cache_p != NULL);
+
     if (bvh_cache_p) {
       BVHCache *bvh_cache = *bvh_cache_p;
       bvhcache_insert(bvh_cache, tree, bvh_cache_type);
@@ -1742,7 +1770,7 @@ BVHTree *BKE_bvhtree_from_pointcloud_get(BVHTreeFromPointCloud *data,
     BLI_bvhtree_insert(tree, i, pointcloud->co[i], 1);
   }
   BLI_assert(BLI_bvhtree_get_len(tree) == pointcloud->totpoint);
-  BLI_bvhtree_balance(tree);
+  bvhtree_balance(tree, false);
 
   data->coords = pointcloud->co;
   data->tree = tree;
diff --git a/source/blender/blenkernel/intern/editmesh_tangent.c b/source/blender/blenkernel/intern/editmesh_tangent.c
index 088a2087a96..d849f4ab37d 100644
--- a/source/blender/blenkernel/intern/editmesh_tangent.c
+++ b/source/blender/blenkernel/intern/editmesh_tangent.c
@@ -362,7 +362,7 @@ void BKE_editmesh_loop_tangent_calc(BMEditMesh *em,
     /* Calculation */
     if (em->tottri != 0) {
       TaskPool *task_pool;
-      task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW, TASK_ISOLATION_ON);
+      task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW);
 
       tangent_mask_curr = 0;
       /* Calculate tangent layers */
diff --git a/source/blender/blenkernel/intern/image.c b/source/blender/blenkernel/intern/image.c
index 2f7e2b41a73..dad518ec696 100644
--- a/source/blender/blenkernel/intern/image.c
+++ b/source/blender/blenkernel/intern/image.c
@@ -68,6 +68,7 @@
 #include "BLI_math_vector.h"
 #include "BLI_mempool.h"
 #include "BLI_system.h"
+#include "BLI_task.h"
 #include "BLI_threads.h"
 #include "BLI_timecode.h" /* For stamp time-code format. */
 #include "BLI_utildefines.h"
@@ -882,6 +883,39 @@ Image *BKE_image_load_exists(Main *bmain, const char *filepath)
   return BKE_image_load_exists_ex(bmain, filepath, NULL);
 }
 
+typedef struct ImageFillData {
+  short gen_type;
+  uint width;
+  uint height;
+  unsigned char *rect;
+  float *rect_float;
+  float fill_color[4];
+} ImageFillData;
+
+static void image_buf_fill_isolated(void *usersata_v)
+{
+  ImageFillData *usersata = usersata_v;
+
+  const short gen_type = usersata->gen_type;
+  const uint width = usersata->width;
+  const uint height = usersata->height;
+
+  unsigned char *rect = usersata->rect;
+  float *rect_float = usersata->rect_float;
+
+  switch (gen_type) {
+    case IMA_GENTYPE_GRID:
+      BKE_image_buf_fill_checker(rect, rect_float, width, height);
+      break;
+    case IMA_GENTYPE_GRID_COLOR:
+      BKE_image_buf_fill_checker_color(rect, rect_float, width, height);
+      break;
+    default:
+      BKE_image_buf_fill_color(rect, rect_float, width, height, usersata->fill_color);
+      break;
+  }
+}
+
 static ImBuf *add_ibuf_size(unsigned int width,
                             unsigned int height,
                             const char *name,
@@ -944,17 +978,16 @@ static ImBuf *add_ibuf_size(unsigned int width,
 
   STRNCPY(ibuf->name, name);
 
-  switch (gen_type) {
-    case IMA_GENTYPE_GRID:
-      BKE_image_buf_fill_checker(rect, rect_float, width, height);
-      break;
-    case IMA_GENTYPE_GRID_COLOR:
-      BKE_image_buf_fill_checker_color(rect, rect_float, width, height);
-      break;
-    default:
-      BKE_image_buf_fill_color(rect, rect_float, width, height, fill_color);
-      break;
-  }
+  ImageFillData data;
+
+  data.gen_type = gen_type;
+  data.width = width;
+  data.height = height;
+  data.rect = rect;
+  data.rect_float = rect_float;
+  copy_v4_v4(data.fill_color, fill_color);
+
+  BLI_task_isolate(image_buf_fill_isolated, &data);
 
   return ibuf;
 }
diff --git a/source/blender/blenkernel/intern/lib_override.c b/source/blender/blenkernel/intern/lib_override.c
index 6b2ffa3b944..c93971e7b11 100644
--- a/source/blender/blenkernel/intern/lib_override.c
+++ b/source/blender/blenkernel/intern/lib_override.c
@@ -2335,8 +2335,7 @@ bool BKE_lib_override_library_main_operations_create(Main *bmain, const bool for
   }
 
   struct LibOverrideOpCreateData create_pool_data = {.bmain = bmain, .changed = false};
-  TaskPool *task_pool = BLI_task_pool_create(
-      &create_pool_data, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON);
+  TaskPool *task_pool = BLI_task_pool_create(&create_pool_data, TASK_PRIORITY_HIGH);
 
   FOREACH_MAIN_ID_BEGIN (bmain, id) {
     if (!ID_IS_LINKED(id) && ID_IS_OVERRIDE_LIBRARY_REAL(id) &&
diff --git a/source/blender/blenkernel/intern/mesh_evaluate.c b/source/blender/blenkernel/intern/mesh_evaluate.c
index 826094420a7..bd79ec4a4bd 100644
--- a/source/blender/blenkernel/intern/mesh_evaluate.c
+++ b/source/blender/blenkernel/intern/mesh_evaluate.c
@@ -1715,8 +1715,7 @@ void BKE_mesh_normals_loop_split(const MVert *mverts,
     loop_split_generator(NULL, &common_data);
   }
   else {
-    TaskPool *task_pool = BLI_task_pool_create(
-        &common_data, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON);
+    TaskPool *task_pool = BLI_task_pool_create(&common_data, TASK_PRIORITY_HIGH);
 
     loop_split_generator(task_pool, &common_data);
 
diff --git a/source/blender/blenkernel/intern/mesh_tangent.c b/source/blender/blenkernel/intern/mesh_tangent.c
index 6a7ff0851f5..2e22e521a13 100644
--- a/source/blender/blenkernel/intern/mesh_tangent.c
+++ b/source/blender/blenkernel/intern/mesh_tangent.c
@@ -656,7 +656,7 @@ void BKE_mesh_calc_loop_tangent_ex(const MVert *mvert,
 
     /* Calculation */
     if (looptri_len != 0) {
-      TaskPool *task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW, TASK_ISOLATION_ON);
+      TaskPool *task_pool = BLI_task_pool_create(NULL, TASK_PRIORITY_LOW);
 
       tangent_mask_curr = 0;
       /* Calculate tangent layers */
diff --git a/source/blender/blenkernel/intern/ocean.c b/source/blender/blenkernel/intern/ocean.c
index 9b9ed0adcf4..9d53dad8d03 100644
--- a/source/blender/blenkernel/intern/ocean.c
+++ b/source/blender/blenkernel/intern/ocean.c
@@ -663,7 +663,7 @@ void BKE_ocean_simulate(struct Ocean *o, float t, float scale, float chop_amount
   osd.scale = scale;
   osd.chop_amount = chop_amount;
 
-  pool = BLI_task_pool_create(&osd, TASK_PRIORITY_HIGH, TASK_ISOLATION_ON);
+  pool = BLI_task_pool_create(&osd, TASK_PRIORITY_HIGH);
 
   BLI_rw_mutex_lock(&o->oceanmutex, THREAD_LOCK_WRITE);
 
diff --git a/source/blender/blenkernel/intern/particle.c b/source/blender/blenkernel/intern/particle.c
index 3ae5d039125..a873ecec6f1 100644
--- a/source/blender/blenkernel/intern/particle.c
+++ b/source/blender/blenkernel/intern/particle.c
@@ -3179,7 +3179,7 @@ void psys_cache_child_paths(ParticleSimulationData *sim,
     return;
   }
 
-  task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW, TASK_ISOLATION_ON);
+  task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW);
   totchild = ctx.totchild;
   totparent = ctx.totparent;
 
diff --git a/source/blender/blenkernel/intern/particle_distribute.c b/source/blender/blenkernel/intern/particle_distribute.c
index 6cae6cd6fa2..ad617b4198b 100644
--- a/source/blender/blenkernel/intern/particle_distribute.c
+++ b/source/blender/blenkernel/intern/particle_distribute.c
@@ -1330,7 +1330,7 @@ static void distribute_particles_on_dm(ParticleSimulationData *sim, int from)
     return;
   }
 
-  task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW, TASK_ISOLATION_ON);
+  task_pool = BLI_task_pool_create(&ctx, TASK_PRIORITY_LOW);
 
   totpart = (from == PART_FROM_CHILD ? sim->psys->totchild : sim->psys->totpart);
   psys_tasks_create(&ctx, 0, totpart, &tasks, &numtasks);
diff --git a/source/blender/blenkernel/intern/volume.cc b/source/blender/blenkernel/intern/volume.cc
index c0ce57818d1..41f0b5c6b72 100644
--- a/source/blender/blenkernel/intern/volume.cc
+++ b/source/blender/blenkernel/intern/volume.cc
@@ -324,15 +324,20 @@ struct VolumeGrid {
 
     openvdb::io::File file(filepath);
 
-    try {
-      file.setCopyMaxBytes(0);
-      file.open();
-      openvdb::GridBase::Ptr vdb_grid = file.readGrid(name());
-      entry->grid->setTree(vdb_grid->baseTreePtr());
-    }
-    catch (const openvdb::IoError &e) {
-      entry->error_msg = e.what();
-    }
+    /* Isolate file loading since that's potentially multithreaded and we are
+     * holding a mutex lock. See BLI_task_isolate. Note OpenVDB already uses
+     * TBB, so it's fine to use here without a wrapper. */
+    tbb::this_task_arena::isolate([&] {
+      try {
+        file.setCopyMaxBytes(0);
+        file.open();
+        openvdb::GridBase::Ptr vdb_grid = file.readGrid(name());
+        entry->grid->setTree(vdb_grid->baseTreePtr());
+      }
+      catch (const openvdb::IoError &e) {
+        entry->error_msg = e.what();
+      }
+    });
 
     std::atomic_thread_fence(std::memory_order_release);
     entry->is_loaded = true;