Fix data race in concurrent lazy initialization (#396)
The first allocation on any thread lazily calls rpmalloc_initialize(0), so when
several threads make their first allocation simultaneously they all enter
rpmalloc_initialize concurrently, racing on the plain-int initialized flag and on
global_config - a thread could be handed a block backed by half-written global
state. Drive initialization as an atomic state machine (UNINIT -> RUNNING -> DONE):
exactly one thread performs the global setup while the others wait for it to finish
before allocating, with release/acquire ordering publishing the global_config writes
to threads that observe completion.
diff --git a/.gitignore b/.gitignore
index 5f51b75..6897419 100644
--- a/.gitignore
+++ b/.gitignore
@@ -238,3 +238,6 @@
# Stress test helper output (test/stress.sh)
/rpmalloc-stress-*
+
+# Init race test helper output (test/init-race.sh)
+/rpmalloc-init-race-*
diff --git a/CHANGELOG b/CHANGELOG
index 50b71e6..6886504 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -1,3 +1,10 @@
+2.0.1
+
+Fixed a data race in concurrent lazy initialization where simultaneous first allocations from
+multiple threads could race on the global init state and return a block backed by a partially
+written configuration. Initialization is now serialized through an atomic state machine.
+
+
2.0.0
Major redesign of the allocator core. The most significant changes in design from the 1.4 series:
diff --git a/rpmalloc/rpmalloc.c b/rpmalloc/rpmalloc.c
index e1e525c..f0cc485 100644
--- a/rpmalloc/rpmalloc.c
+++ b/rpmalloc/rpmalloc.c
@@ -641,8 +641,16 @@
static atomic_uintptr_t global_heap_lock;
//! Heap ID counter
static atomic_uint global_heap_id = 1;
-//! Initialized flag
-static int global_rpmalloc_initialized;
+//! Global initialization state. rpmalloc_initialize can be called concurrently from multiple
+//! threads (the first allocation on any thread may trigger it), so it is driven as an atomic
+//! state machine: exactly one thread performs the global setup while the others wait for it to
+//! complete before allocating - they must never observe, or allocate against, a partially
+//! written global_config. Release/acquire ordering on the DONE transition publishes all of the
+//! global_config writes to threads that observe completion.
+#define RPMALLOC_INIT_UNINIT 0
+#define RPMALLOC_INIT_RUNNING 1
+#define RPMALLOC_INIT_DONE 2
+static atomic_int global_rpmalloc_init_state;
//! Memory interface
static rpmalloc_interface_t* global_memory_interface;
//! Default memory interface
@@ -2666,7 +2674,7 @@
extern int
rpmalloc_initialize_config(rpmalloc_interface_t* memory_interface, rpmalloc_config_t* config) {
- if (global_rpmalloc_initialized) {
+ if (atomic_load_explicit(&global_rpmalloc_init_state, memory_order_acquire) == RPMALLOC_INIT_DONE) {
rpmalloc_thread_initialize();
if (config)
*config = global_config;
@@ -2686,13 +2694,25 @@
extern int
rpmalloc_initialize(rpmalloc_interface_t* memory_interface) {
- if (global_rpmalloc_initialized) {
- rpmalloc_thread_initialize();
- return 0;
+ for (;;) {
+ int state = atomic_load_explicit(&global_rpmalloc_init_state, memory_order_acquire);
+ if (state == RPMALLOC_INIT_DONE) {
+ rpmalloc_thread_initialize();
+ return 0;
+ }
+ if (state == RPMALLOC_INIT_RUNNING) {
+ while (atomic_load_explicit(&global_rpmalloc_init_state, memory_order_acquire) ==
+ RPMALLOC_INIT_RUNNING)
+ wait_spin();
+ continue;
+ }
+ int expected = RPMALLOC_INIT_UNINIT;
+ if (atomic_compare_exchange_strong_explicit(&global_rpmalloc_init_state, &expected,
+ RPMALLOC_INIT_RUNNING, memory_order_acq_rel,
+ memory_order_acquire))
+ break;
}
- global_rpmalloc_initialized = 1;
-
// Remember whether the caller explicitly requested huge pages, the detection below
// overwrites global_config.enable_huge_pages with what is actually available.
const int huge_pages_requested = global_config.enable_huge_pages;
@@ -2850,7 +2870,7 @@
// huge page size. Leave the allocator uninitialized (and the requested flag cleared)
// so the caller can detect this and re-initialize without huge pages if desired.
global_config.enable_huge_pages = 0;
- global_rpmalloc_initialized = 0;
+ atomic_store_explicit(&global_rpmalloc_init_state, RPMALLOC_INIT_UNINIT, memory_order_release);
return -1;
}
@@ -2908,6 +2928,8 @@
global_main_thread_id = get_thread_id();
+ atomic_store_explicit(&global_rpmalloc_init_state, RPMALLOC_INIT_DONE, memory_order_release);
+
rpmalloc_thread_initialize();
return 0;
@@ -3055,7 +3077,7 @@
global_heap_creating = 0;
global_main_thread_id = 0;
- global_rpmalloc_initialized = 0;
+ atomic_store_explicit(&global_rpmalloc_init_state, RPMALLOC_INIT_UNINIT, memory_order_release);
}
extern void
diff --git a/rpmalloc/rpmalloc.h b/rpmalloc/rpmalloc.h
index 7d4c86e..565c511 100644
--- a/rpmalloc/rpmalloc.h
+++ b/rpmalloc/rpmalloc.h
@@ -19,10 +19,10 @@
//! rpmalloc version. RPMALLOC_VERSION is a human-readable string and may carry a pre-release
// suffix such as "-rc1". RPMALLOC_VERSION_NUMBER is a monotonic integer for comparisons,
// computed as major*10000 + minor*100 + patch (pre-release suffixes are not encoded).
-#define RPMALLOC_VERSION "2.0.0"
+#define RPMALLOC_VERSION "2.0.1"
#define RPMALLOC_VERSION_MAJOR 2
#define RPMALLOC_VERSION_MINOR 0
-#define RPMALLOC_VERSION_PATCH 0
+#define RPMALLOC_VERSION_PATCH 1
#define RPMALLOC_VERSION_NUMBER \
(RPMALLOC_VERSION_MAJOR * 10000 + RPMALLOC_VERSION_MINOR * 100 + RPMALLOC_VERSION_PATCH)
