Cleanup: make guarded memory allocation always thread safe

Previously this would be enabled when threads were used, but threads are now
basically always in use so there is no point. Further, this is only needed for
guarded allocation with --debug-memory which is not performance critical.

15 files changed, 7 insertions, 164 deletions

diff --git a/intern/guardedalloc/MEM_guardedalloc.h b/intern/guardedalloc/MEM_guardedalloc.h
index f4fcebf6811..602297576c8 100644
--- a/intern/guardedalloc/MEM_guardedalloc.h
+++ b/intern/guardedalloc/MEM_guardedalloc.h
@@ -168,10 +168,6 @@ extern void (*MEM_set_error_callback)(void (*func)(const char *));
  * @retval true for correct memory, false for corrupted memory. */
 extern bool (*MEM_consistency_check)(void);
 
-/** Set thread locking functions for safe memory allocation from multiple
- * threads, pass NULL pointers to disable thread locking again. */
-extern void (*MEM_set_lock_callback)(void (*lock)(void), void (*unlock)(void));
-
 /** Attempt to enforce OSX (or other OS's) to have malloc and stack nonzero */
 extern void (*MEM_set_memory_debug)(void);
 
diff --git a/intern/guardedalloc/intern/mallocn.c b/intern/guardedalloc/intern/mallocn.c
index 82a8aa3eb21..e85f8eb03ed 100644
--- a/intern/guardedalloc/intern/mallocn.c
+++ b/intern/guardedalloc/intern/mallocn.c
@@ -54,8 +54,6 @@ void (*MEM_callbackmemlist)(void (*func)(void *)) = MEM_lockfree_callbackmemlist
 void (*MEM_printmemlist_stats)(void) = MEM_lockfree_printmemlist_stats;
 void (*MEM_set_error_callback)(void (*func)(const char *)) = MEM_lockfree_set_error_callback;
 bool (*MEM_consistency_check)(void) = MEM_lockfree_consistency_check;
-void (*MEM_set_lock_callback)(void (*lock)(void),
-                              void (*unlock)(void)) = MEM_lockfree_set_lock_callback;
 void (*MEM_set_memory_debug)(void) = MEM_lockfree_set_memory_debug;
 size_t (*MEM_get_memory_in_use)(void) = MEM_lockfree_get_memory_in_use;
 unsigned int (*MEM_get_memory_blocks_in_use)(void) = MEM_lockfree_get_memory_blocks_in_use;
@@ -115,7 +113,6 @@ void MEM_use_guarded_allocator(void)
   MEM_printmemlist_stats = MEM_guarded_printmemlist_stats;
   MEM_set_error_callback = MEM_guarded_set_error_callback;
   MEM_consistency_check = MEM_guarded_consistency_check;
-  MEM_set_lock_callback = MEM_guarded_set_lock_callback;
   MEM_set_memory_debug = MEM_guarded_set_memory_debug;
   MEM_get_memory_in_use = MEM_guarded_get_memory_in_use;
   MEM_get_memory_blocks_in_use = MEM_guarded_get_memory_blocks_in_use;
diff --git a/intern/guardedalloc/intern/mallocn_guarded_impl.c b/intern/guardedalloc/intern/mallocn_guarded_impl.c
index 8aa9fc767dd..20dcbed7235 100644
--- a/intern/guardedalloc/intern/mallocn_guarded_impl.c
+++ b/intern/guardedalloc/intern/mallocn_guarded_impl.c
@@ -28,6 +28,8 @@
 #include <string.h> /* memcpy */
 #include <sys/types.h>
 
+#include <pthread.h>
+
 #include "MEM_guardedalloc.h"
 
 /* to ensure strict conversions */
@@ -51,17 +53,6 @@
 //#define DEBUG_MEMCOUNTER
 
 /* Only for debugging:
- * defining DEBUG_THREADS will enable check whether memory manager
- * is locked with a mutex when allocation is called from non-main
- * thread.
- *
- * This helps troubleshooting memory issues caused by the fact
- * guarded allocator is not thread-safe, however this check will
- * fail to check allocations from openmp threads.
- */
-//#define DEBUG_THREADS
-
-/* Only for debugging:
  * Defining DEBUG_BACKTRACE will store a backtrace from where
  * memory block was allocated and print this trace for all
  * unfreed blocks.
@@ -124,24 +115,6 @@ typedef struct MemHead {
 
 typedef MemHead MemHeadAligned;
 
-/* for openmp threading asserts, saves time troubleshooting
- * we may need to extend this if blender code starts using MEM_
- * functions inside OpenMP correctly with omp_set_lock() */
-
-#if 0 /* disable for now, only use to debug openmp code which doesn lock threads for malloc */
-#  if defined(_OPENMP) && defined(DEBUG)
-#    include <assert.h>
-#    include <omp.h>
-#    define DEBUG_OMP_MALLOC
-#  endif
-#endif
-
-#ifdef DEBUG_THREADS
-#  include <assert.h>
-#  include <pthread.h>
-static pthread_t mainid;
-#endif
-
 #ifdef DEBUG_BACKTRACE
 #  if defined(__linux__) || defined(__APPLE__)
 #    include <execinfo.h>
@@ -192,8 +165,6 @@ static size_t mem_in_use = 0, peak_mem = 0;
 static volatile struct localListBase _membase;
 static volatile struct localListBase *membase = &_membase;
 static void (*error_callback)(const char *) = NULL;
-static void (*thread_lock_callback)(void) = NULL;
-static void (*thread_unlock_callback)(void) = NULL;
 
 static bool malloc_debug_memset = false;
 
@@ -233,40 +204,16 @@ print_error(const char *str, ...)
     fputs(buf, stderr);
 }
 
+static pthread_mutex_t thread_lock = PTHREAD_MUTEX_INITIALIZER;
+
 static void mem_lock_thread(void)
 {
-#ifdef DEBUG_THREADS
-  static int initialized = 0;
-
-  if (initialized == 0) {
-    /* assume first allocation happens from main thread */
-    mainid = pthread_self();
-    initialized = 1;
-  }
-
-  if (!pthread_equal(pthread_self(), mainid) && thread_lock_callback == NULL) {
-    assert(!"Memory function is called from non-main thread without lock");
-  }
-#endif
-
-#ifdef DEBUG_OMP_MALLOC
-  assert(omp_in_parallel() == 0);
-#endif
-
-  if (thread_lock_callback)
-    thread_lock_callback();
+  pthread_mutex_lock(&thread_lock);
 }
 
 static void mem_unlock_thread(void)
 {
-#ifdef DEBUG_THREADS
-  if (!pthread_equal(pthread_self(), mainid) && thread_lock_callback == NULL) {
-    assert(!"Thread lock was removed while allocation from thread is in progress");
-  }
-#endif
-
-  if (thread_unlock_callback)
-    thread_unlock_callback();
+  pthread_mutex_unlock(&thread_lock);
 }
 
 bool MEM_guarded_consistency_check(void)
@@ -287,12 +234,6 @@ void MEM_guarded_set_error_callback(void (*func)(const char *))
   error_callback = func;
 }
 
-void MEM_guarded_set_lock_callback(void (*lock)(void), void (*unlock)(void))
-{
-  thread_lock_callback = lock;
-  thread_unlock_callback = unlock;
-}
-
 void MEM_guarded_set_memory_debug(void)
 {
   malloc_debug_memset = true;
diff --git a/intern/guardedalloc/intern/mallocn_intern.h b/intern/guardedalloc/intern/mallocn_intern.h
index 6e8c580e0ad..ef8845a66b3 100644
--- a/intern/guardedalloc/intern/mallocn_intern.h
+++ b/intern/guardedalloc/intern/mallocn_intern.h
@@ -139,7 +139,6 @@ void MEM_lockfree_callbackmemlist(void (*func)(void *));
 void MEM_lockfree_printmemlist_stats(void);
 void MEM_lockfree_set_error_callback(void (*func)(const char *));
 bool MEM_lockfree_consistency_check(void);
-void MEM_lockfree_set_lock_callback(void (*lock)(void), void (*unlock)(void));
 void MEM_lockfree_set_memory_debug(void);
 size_t MEM_lockfree_get_memory_in_use(void);
 unsigned int MEM_lockfree_get_memory_blocks_in_use(void);
@@ -183,7 +182,6 @@ void MEM_guarded_callbackmemlist(void (*func)(void *));
 void MEM_guarded_printmemlist_stats(void);
 void MEM_guarded_set_error_callback(void (*func)(const char *));
 bool MEM_guarded_consistency_check(void);
-void MEM_guarded_set_lock_callback(void (*lock)(void), void (*unlock)(void));
 void MEM_guarded_set_memory_debug(void);
 size_t MEM_guarded_get_memory_in_use(void);
 unsigned int MEM_guarded_get_memory_blocks_in_use(void);
diff --git a/intern/guardedalloc/intern/mallocn_lockfree_impl.c b/intern/guardedalloc/intern/mallocn_lockfree_impl.c
index 7b8b405b372..205cc688d72 100644
--- a/intern/guardedalloc/intern/mallocn_lockfree_impl.c
+++ b/intern/guardedalloc/intern/mallocn_lockfree_impl.c
@@ -400,12 +400,6 @@ bool MEM_lockfree_consistency_check(void)
   return true;
 }
 
-void MEM_lockfree_set_lock_callback(void (*lock)(void), void (*unlock)(void))
-{
-  (void)lock;
-  (void)unlock;
-}
-
 void MEM_lockfree_set_memory_debug(void)
 {
   malloc_debug_memset = true;
diff --git a/source/blender/blenkernel/intern/dynamicpaint.c b/source/blender/blenkernel/intern/dynamicpaint.c
index 79d9a40f06b..f3cc17f46f6 100644
--- a/source/blender/blenkernel/intern/dynamicpaint.c
+++ b/source/blender/blenkernel/intern/dynamicpaint.c
@@ -4646,9 +4646,6 @@ static int dynamicPaint_paintParticles(DynamicPaintSurface *surface,
     return 1;
   }
 
-  /* begin thread safe malloc */
-  BLI_threaded_malloc_begin();
-
   /* only continue if particle bb is close enough to canvas bb */
   if (boundsIntersectDist(&grid->grid_bounds, &part_bb, range)) {
     int c_index;
@@ -4684,7 +4681,6 @@ static int dynamicPaint_paintParticles(DynamicPaintSurface *surface,
                               &settings);
     }
   }
-  BLI_threaded_malloc_end();
   BLI_kdtree_3d_free(tree);
 
   return 1;
diff --git a/source/blender/blenlib/BLI_threads.h b/source/blender/blenlib/BLI_threads.h
index c199417017b..03fe27c10ed 100644
--- a/source/blender/blenlib/BLI_threads.h
+++ b/source/blender/blenlib/BLI_threads.h
@@ -58,9 +58,6 @@ void BLI_threadpool_clear(struct ListBase *threadbase);
 void BLI_threadpool_end(struct ListBase *threadbase);
 int BLI_thread_is_main(void);
 
-void BLI_threaded_malloc_begin(void);
-void BLI_threaded_malloc_end(void);
-
 /* System Information */
 
 int BLI_system_thread_count(void); /* gets the number of threads the system can make use of */
diff --git a/source/blender/blenlib/intern/task_pool.cc b/source/blender/blenlib/intern/task_pool.cc
index 670787697a3..cf328ec407c 100644
--- a/source/blender/blenlib/intern/task_pool.cc
+++ b/source/blender/blenlib/intern/task_pool.cc
@@ -364,14 +364,6 @@ static void background_task_pool_free(TaskPool *pool)
 
 static TaskPool *task_pool_create_ex(void *userdata, TaskPoolType type, TaskPriority priority)
 {
-  /* Ensure malloc will go fine from threads,
-   *
-   * This is needed because we could be in main thread here
-   * and malloc could be non-thread safe at this point because
-   * no other jobs are running.
-   */
-  BLI_threaded_malloc_begin();
-
   const bool use_threads = BLI_task_scheduler_num_threads() > 1 && type != TASK_POOL_NO_THREADS;
 
   /* Background task pool uses regular TBB scheduling if available. Only when
@@ -475,8 +467,6 @@ void BLI_task_pool_free(TaskPool *pool)
   BLI_mutex_end(&pool->user_mutex);
 
   MEM_freeN(pool);
-
-  BLI_threaded_malloc_end();
 }
 
 void BLI_task_pool_push(TaskPool *pool,
diff --git a/source/blender/blenlib/intern/task_range.cc b/source/blender/blenlib/intern/task_range.cc
index da38c8fd352..67d8960434e 100644
--- a/source/blender/blenlib/intern/task_range.cc
+++ b/source/blender/blenlib/intern/task_range.cc
@@ -115,8 +115,6 @@ void BLI_task_parallel_range(const int start,
 #ifdef WITH_TBB
   /* Multithreading. */
   if (settings->use_threading && BLI_task_scheduler_num_threads() > 1) {
-    BLI_threaded_malloc_begin();
-
     RangeTask task(func, userdata, settings);
     const size_t grainsize = MAX2(settings->min_iter_per_thread, 1);
     const tbb::blocked_range<int> range(start, stop, grainsize);
@@ -130,8 +128,6 @@ void BLI_task_parallel_range(const int start,
     else {
       parallel_for(range, task);
     }
-
-    BLI_threaded_malloc_end();
     return;
   }
 #endif
diff --git a/source/blender/blenlib/intern/threads.c b/source/blender/blenlib/intern/threads.c
index f535798f86d..be43c27e945 100644
--- a/source/blender/blenlib/intern/threads.c
+++ b/source/blender/blenlib/intern/threads.c
@@ -104,7 +104,6 @@ static void *thread_tls_data;
  *     BLI_threadpool_end(&lb);
  *
  ************************************************ */
-static SpinLock _malloc_lock;
 static pthread_mutex_t _image_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _image_draw_lock = PTHREAD_MUTEX_INITIALIZER;
 static pthread_mutex_t _viewer_lock = PTHREAD_MUTEX_INITIALIZER;
@@ -132,21 +131,9 @@ typedef struct ThreadSlot {
   int avail;
 } ThreadSlot;
 
-static void BLI_lock_malloc_thread(void)
-{
-  BLI_spin_lock(&_malloc_lock);
-}
-
-static void BLI_unlock_malloc_thread(void)
-{
-  BLI_spin_unlock(&_malloc_lock);
-}
-
 void BLI_threadapi_init(void)
 {
   mainid = pthread_self();
-
-  BLI_spin_init(&_malloc_lock);
   if (numaAPI_Initialize() == NUMAAPI_SUCCESS) {
     is_numa_available = true;
   }
@@ -154,7 +141,6 @@ void BLI_threadapi_init(void)
 
 void BLI_threadapi_exit(void)
 {
-  BLI_spin_end(&_malloc_lock);
 }
 
 /* tot = 0 only initializes malloc mutex in a safe way (see sequence.c)
@@ -185,8 +171,6 @@ void BLI_threadpool_init(ListBase *threadbase, void *(*do_thread)(void *), int t
 
   unsigned int level = atomic_fetch_and_add_u(&thread_levels, 1);
   if (level == 0) {
-    MEM_set_lock_callback(BLI_lock_malloc_thread, BLI_unlock_malloc_thread);
-
 #ifdef USE_APPLE_OMP_FIX
     /* workaround for Apple gcc 4.2.1 omp vs background thread bug,
      * we copy gomp thread local storage pointer to setting it again
@@ -313,11 +297,6 @@ void BLI_threadpool_end(ListBase *threadbase)
     }
     BLI_freelistN(threadbase);
   }
-
-  unsigned int level = atomic_sub_and_fetch_u(&thread_levels, 1);
-  if (level == 0) {
-    MEM_set_lock_callback(NULL, NULL);
-  }
 }
 
 /* System Information */
@@ -811,24 +790,6 @@ void BLI_thread_queue_wait_finish(ThreadQueue *queue)
   pthread_mutex_unlock(&queue->mutex);
 }
 
-/* ************************************************ */
-
-void BLI_threaded_malloc_begin(void)
-{
-  unsigned int level = atomic_fetch_and_add_u(&thread_levels, 1);
-  if (level == 0) {
-    MEM_set_lock_callback(BLI_lock_malloc_thread, BLI_unlock_malloc_thread);
-  }
-}
-
-void BLI_threaded_malloc_end(void)
-{
-  unsigned int level = atomic_sub_and_fetch_u(&thread_levels, 1);
-  if (level == 0) {
-    MEM_set_lock_callback(NULL, NULL);
-  }
-}
-
 /* **** Special functions to help performance on crazy NUMA setups. **** */
 
 #if 0  /* UNUSED */
diff --git a/source/blender/draw/engines/external/external_engine.c b/source/blender/draw/engines/external/external_engine.c
index 7dddbd07623..6e636e4dc93 100644
--- a/source/blender/draw/engines/external/external_engine.c
+++ b/source/blender/draw/engines/external/external_engine.c
@@ -215,7 +215,7 @@ static void external_draw_scene_do(void *vedata)
       return;
     }
 
-    RenderEngine *engine = RE_engine_create_ex(engine_type, true);
+    RenderEngine *engine = RE_engine_create(engine_type);
     engine->tile_x = scene->r.tilex;
     engine->tile_y = scene->r.tiley;
     engine_type->view_update(engine, draw_ctx->evil_C, draw_ctx->depsgraph);
diff --git a/source/blender/editors/render/render_internal.c b/source/blender/editors/render/render_internal.c
index 6d7ed73f221..46827940b57 100644
--- a/source/blender/editors/render/render_internal.c
+++ b/source/blender/editors/render/render_internal.c
@@ -349,7 +349,6 @@ static int screen_render_exec(bContext *C, wmOperator *op)
 
   RE_SetReports(re, op->reports);
 
-  BLI_threaded_malloc_begin();
   if (is_animation) {
     RE_RenderAnim(re,
                   mainp,
@@ -363,7 +362,6 @@ static int screen_render_exec(bContext *C, wmOperator *op)
   else {
     RE_RenderFrame(re, mainp, scene, single_layer, camera_override, scene->r.cfra, is_write_still);
   }
-  BLI_threaded_malloc_end();
 
   RE_SetReports(re, NULL);
 
diff --git a/source/blender/render/extern/include/RE_engine.h b/source/blender/render/extern/include/RE_engine.h
index 82b45ba9d4a..77a60854616 100644
--- a/source/blender/render/extern/include/RE_engine.h
+++ b/source/blender/render/extern/include/RE_engine.h
@@ -68,7 +68,6 @@ struct bNodeTree;
 #define RE_ENGINE_DO_UPDATE 8
 #define RE_ENGINE_RENDERING 16
 #define RE_ENGINE_HIGHLIGHT_TILES 32
-#define RE_ENGINE_USED_FOR_VIEWPORT 64
 
 extern ListBase R_engines;
 
@@ -159,7 +158,6 @@ typedef struct RenderEngine {
 } RenderEngine;
 
 RenderEngine *RE_engine_create(RenderEngineType *type);
-RenderEngine *RE_engine_create_ex(RenderEngineType *type, bool use_for_viewport);
 void RE_engine_free(RenderEngine *engine);
 
 void RE_layer_load_from_file(
diff --git a/source/blender/render/intern/source/external_engine.c b/source/blender/render/intern/source/external_engine.c
index 4d88bb82dd9..cc685610886 100644
--- a/source/blender/render/intern/source/external_engine.c
+++ b/source/blender/render/intern/source/external_engine.c
@@ -132,20 +132,9 @@ bool RE_engine_is_opengl(RenderEngineType *render_type)
 
 RenderEngine *RE_engine_create(RenderEngineType *type)
 {
-  return RE_engine_create_ex(type, false);
-}
-
-RenderEngine *RE_engine_create_ex(RenderEngineType *type, bool use_for_viewport)
-{
   RenderEngine *engine = MEM_callocN(sizeof(RenderEngine), "RenderEngine");
   engine->type = type;
 
-  if (use_for_viewport) {
-    engine->flag |= RE_ENGINE_USED_FOR_VIEWPORT;
-
-    BLI_threaded_malloc_begin();
-  }
-
   BLI_mutex_init(&engine->update_render_passes_mutex);
 
   return engine;
@@ -159,10 +148,6 @@ void RE_engine_free(RenderEngine *engine)
   }
 #endif
 
-  if (engine->flag & RE_ENGINE_USED_FOR_VIEWPORT) {
-    BLI_threaded_malloc_end();
-  }
-
   BLI_mutex_end(&engine->update_render_passes_mutex);
 
   MEM_freeN(engine);
diff --git a/source/creator/creator_args.c b/source/creator/creator_args.c
index 88c5cc9e53d..7acf649c3ab 100644
--- a/source/creator/creator_args.c
+++ b/source/creator/creator_args.c
@@ -1592,7 +1592,6 @@ static int arg_handle_render_frame(int argc, const char **argv, void *data)
       }
 
       re = RE_NewSceneRender(scene);
-      BLI_threaded_malloc_begin();
       BKE_reports_init(&reports, RPT_STORE);
       RE_SetReports(re, &reports);
       for (int i = 0; i < frames_range_len; i++) {
@@ -1608,7 +1607,6 @@ static int arg_handle_render_frame(int argc, const char **argv, void *data)
       }
       RE_SetReports(re, NULL);
       BKE_reports_clear(&reports);
-      BLI_threaded_malloc_end();
       MEM_freeN(frame_range_arr);
       return 1;
     }
@@ -1634,13 +1632,11 @@ static int arg_handle_render_animation(int UNUSED(argc), const char **UNUSED(arg
     Main *bmain = CTX_data_main(C);
     Render *re = RE_NewSceneRender(scene);
     ReportList reports;
-    BLI_threaded_malloc_begin();
     BKE_reports_init(&reports, RPT_STORE);
     RE_SetReports(re, &reports);
     RE_RenderAnim(re, bmain, scene, NULL, NULL, scene->r.sfra, scene->r.efra, scene->r.frame_step);
     RE_SetReports(re, NULL);
     BKE_reports_clear(&reports);
-    BLI_threaded_malloc_end();
   }
   else {
     printf("\nError: no blend loaded. cannot use '-a'.\n");