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.

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);
 

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;

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 */
@@ -50,17 +52,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
@@ -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;

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);

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;

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;

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 */

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,

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

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 */

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);

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);
 

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(

source/blender/render/intern/source/external_engine.c

@@ -131,21 +131,10 @@ bool RE_engine_is_opengl(RenderEngineType *render_type)
 /* Create, Free */
 
 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);

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");