From 4caf3f8b94a38e738314f484d18b531dfa16ff8a Mon Sep 17 00:00:00 2001
From: Sverker Eriksson <sverker@erlang.org>
Date: Mon, 15 Apr 2013 14:07:42 +0200
Subject: erts: Fix locking order violation for allocation wrappers

Some query functions in erl_alloc_util.c lock the allocator mutex
and then use erts_printf that in turn may call the sys allocator
through the wrappers. To avoid breaking locking order these
query functions first "pre-locks" all allocator wrappers.
---
 erts/emulator/beam/erl_alloc_util.c | 66 ++++++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 30 deletions(-)

(limited to 'erts/emulator/beam/erl_alloc_util.c')

diff --git a/erts/emulator/beam/erl_alloc_util.c b/erts/emulator/beam/erl_alloc_util.c
index 583aeed12d..cbeb2c1e3a 100644
--- a/erts/emulator/beam/erl_alloc_util.c
+++ b/erts/emulator/beam/erl_alloc_util.c
@@ -588,17 +588,21 @@ do {									\
 
 #ifdef DEBUG
 #ifdef USE_THREADS
+# ifdef ERTS_SMP
+#  define IS_ACTUALLY_BLOCKING (erts_thr_progress_is_blocking())
+# else
+#  define IS_ACTUALLY_BLOCKING 0
+# endif
 #define ERTS_ALCU_DBG_CHK_THR_ACCESS(A)					\
 do {									\
-    if (!(A)->thread_safe) {						\
-	if (!(A)->debug.saved_tid) {					\
+    if (!(A)->thread_safe && !IS_ACTUALLY_BLOCKING) {                   \
+	if (!(A)->debug.saved_tid) {                                    \
 	    (A)->debug.tid = erts_thr_self();				\
 	    (A)->debug.saved_tid = 1;					\
 	}								\
 	else {								\
 	    ERTS_SMP_LC_ASSERT(						\
-		ethr_equal_tids((A)->debug.tid, erts_thr_self())	\
-		|| erts_thr_progress_is_blocking());			\
+		ethr_equal_tids((A)->debug.tid, erts_thr_self()));	\
 	}								\
     }									\
 } while (0)
@@ -2510,12 +2514,6 @@ static erts_mtx_t init_atoms_mtx;
 static void
 init_atoms(Allctr_t *allctr)
 {
-
-#ifdef USE_THREADS
-    if (allctr && allctr->thread_safe)
-	erts_mtx_unlock(&allctr->mutex);
-#endif
-
     erts_mtx_lock(&init_atoms_mtx);
 
     if (!atoms_initialized) {
@@ -2606,18 +2604,13 @@ init_atoms(Allctr_t *allctr)
 	    fix_type_atoms[ix] = am_atom_put(name, len);
 	}
     }
-
     
-    if (allctr) {
+    if (allctr && !allctr->atoms_initialized) {
 
 	make_name_atoms(allctr);
 
 	(*allctr->init_atoms)();
 
-#ifdef USE_THREADS
-	if (allctr->thread_safe)
-	    erts_mtx_lock(&allctr->mutex);
-#endif
     	allctr->atoms_initialized = 1;
     }
 
@@ -3245,17 +3238,21 @@ erts_alcu_info_options(Allctr_t *allctr,
 {
     Eterm res;
 
+    if (hpp || szp)
+	ensure_atoms_initialized(allctr);
 
 #ifdef USE_THREADS
-    if (allctr->thread_safe)
+    if (allctr->thread_safe) {
+	erts_allctr_wrapper_pre_lock();
 	erts_mtx_lock(&allctr->mutex);
+    }
 #endif
-    if (hpp || szp)
-	ensure_atoms_initialized(allctr);
     res = info_options(allctr, print_to_p, print_to_arg, hpp, szp);
 #ifdef USE_THREADS
-    if (allctr->thread_safe)
+    if (allctr->thread_safe) { 
 	erts_mtx_unlock(&allctr->mutex);
+	erts_allctr_wrapper_pre_unlock();
+    }
 #endif
     return res;
 }
@@ -3282,16 +3279,18 @@ erts_alcu_sz_info(Allctr_t *allctr,
 	return am_false;
     }
 
+    if (hpp || szp)
+	ensure_atoms_initialized(allctr);
+
 #ifdef USE_THREADS
-    if (allctr->thread_safe)
+    if (allctr->thread_safe) {
+	erts_allctr_wrapper_pre_lock();
 	erts_mtx_lock(&allctr->mutex);
+    }
 #endif
 
     ERTS_ALCU_DBG_CHK_THR_ACCESS(allctr);
 
-    if (hpp || szp)
-	ensure_atoms_initialized(allctr);
-
     /* Update sbc values not continously updated */
     allctr->sbcs.blocks.curr.no
 	= allctr->sbcs.curr.norm.mseg.no + allctr->sbcs.curr.norm.sys_alloc.no;
@@ -3327,13 +3326,16 @@ erts_alcu_sz_info(Allctr_t *allctr,
 
 
 #ifdef USE_THREADS
-    if (allctr->thread_safe)
+    if (allctr->thread_safe) {
 	erts_mtx_unlock(&allctr->mutex);
+	erts_allctr_wrapper_pre_unlock();
+    }
 #endif
 
     return res;
 }
 
+
 Eterm
 erts_alcu_info(Allctr_t *allctr,
 	       int begin_max_period,
@@ -3354,16 +3356,18 @@ erts_alcu_info(Allctr_t *allctr,
 	return am_false;
     }
 
+    if (hpp || szp)
+	ensure_atoms_initialized(allctr);
+
 #ifdef USE_THREADS
-    if (allctr->thread_safe)
+    if (allctr->thread_safe) {
+	erts_allctr_wrapper_pre_lock();
 	erts_mtx_lock(&allctr->mutex);
+    }
 #endif
 
     ERTS_ALCU_DBG_CHK_THR_ACCESS(allctr);
 
-    if (hpp || szp)
-	ensure_atoms_initialized(allctr);
-
     /* Update sbc values not continously updated */
     allctr->sbcs.blocks.curr.no
 	= allctr->sbcs.curr.norm.mseg.no + allctr->sbcs.curr.norm.sys_alloc.no;
@@ -3416,8 +3420,10 @@ erts_alcu_info(Allctr_t *allctr,
 
 
 #ifdef USE_THREADS
-    if (allctr->thread_safe)
+    if (allctr->thread_safe) {
 	erts_mtx_unlock(&allctr->mutex);
+	erts_allctr_wrapper_pre_unlock();
+    }
 #endif
 
     return res;
-- 
cgit v1.2.3