Skip to content

Commit 73713fb

Browse files
committed
Drop high rank locks when creating threads.
Avoid holding arenas_lock and background_thread_lock when creating background threads, because pthread_create may take internal locks, and potentially cause deadlock with jemalloc internal locks.
1 parent 00869e3 commit 73713fb

File tree

5 files changed

+43
-13
lines changed

5 files changed

+43
-13
lines changed

include/jemalloc/internal/arena_externs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ extern percpu_arena_mode_t opt_percpu_arena;
1515
extern const char *percpu_arena_mode_names[];
1616

1717
extern const uint64_t h_steps[SMOOTHSTEP_NSTEPS];
18+
extern malloc_mutex_t arenas_lock;
1819

1920
void arena_stats_large_nrequests_add(tsdn_t *tsdn, arena_stats_t *arena_stats,
2021
szind_t szind, uint64_t nrequests);

src/arena.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2050,17 +2050,6 @@ arena_new(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) {
20502050
hooks_arena_new_hook();
20512051
}
20522052
post_reentrancy(tsdn_tsd(tsdn));
2053-
2054-
/* background_thread_create() handles reentrancy internally. */
2055-
if (have_background_thread) {
2056-
bool err;
2057-
malloc_mutex_lock(tsdn, &background_thread_lock);
2058-
err = background_thread_create(tsdn_tsd(tsdn), ind);
2059-
malloc_mutex_unlock(tsdn, &background_thread_lock);
2060-
if (err) {
2061-
goto label_error;
2062-
}
2063-
}
20642053
}
20652054

20662055
return arena;

src/background_thread.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,12 +352,15 @@ background_thread_create(tsd_t *tsd, unsigned arena_ind) {
352352
}
353353

354354
pre_reentrancy(tsd);
355+
malloc_mutex_unlock(tsd_tsdn(tsd), &background_thread_lock);
355356
/*
356357
* To avoid complications (besides reentrancy), create internal
357-
* background threads with the underlying pthread_create.
358+
* background threads with the underlying pthread_create, and drop
359+
* background_thread_lock (pthread_create may take internal locks).
358360
*/
359361
int err = pthread_create_wrapper(&info->thread, NULL,
360362
background_thread_entry, (void *)thread_ind);
363+
malloc_mutex_lock(tsd_tsdn(tsd), &background_thread_lock);
361364
post_reentrancy(tsd);
362365

363366
if (err != 0) {

src/ctl.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,6 +1501,7 @@ background_thread_ctl(tsd_t *tsd, const size_t *mib, size_t miblen,
15011501
}
15021502
background_thread_ctl_init(tsd_tsdn(tsd));
15031503

1504+
malloc_mutex_lock(tsd_tsdn(tsd), &ctl_mtx);
15041505
malloc_mutex_lock(tsd_tsdn(tsd), &background_thread_lock);
15051506
if (newp == NULL) {
15061507
oldval = background_thread_enabled();
@@ -1535,6 +1536,8 @@ background_thread_ctl(tsd_t *tsd, const size_t *mib, size_t miblen,
15351536
ret = 0;
15361537
label_return:
15371538
malloc_mutex_unlock(tsd_tsdn(tsd), &background_thread_lock);
1539+
malloc_mutex_unlock(tsd_tsdn(tsd), &ctl_mtx);
1540+
15381541
return ret;
15391542
}
15401543

src/jemalloc.c

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ unsigned opt_narenas = 0;
7070
unsigned ncpus;
7171

7272
/* Protects arenas initialization. */
73-
static malloc_mutex_t arenas_lock;
73+
malloc_mutex_t arenas_lock;
7474
/*
7575
* Arenas that are used to service external requests. Not all elements of the
7676
* arenas array are necessarily used; arenas are created lazily as needed.
@@ -335,13 +335,35 @@ arena_init_locked(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) {
335335
return arena;
336336
}
337337

338+
static void
339+
arena_new_create_background_thread(tsdn_t *tsdn, unsigned ind) {
340+
if (ind == 0) {
341+
return;
342+
}
343+
/* background_thread_create() handles reentrancy internally. */
344+
if (have_background_thread) {
345+
bool err;
346+
malloc_mutex_lock(tsdn, &background_thread_lock);
347+
err = background_thread_create(tsdn_tsd(tsdn), ind);
348+
malloc_mutex_unlock(tsdn, &background_thread_lock);
349+
if (err) {
350+
malloc_printf("<jemalloc>: error in background thread "
351+
"creation for arena %u. Abort.\n", ind);
352+
abort();
353+
}
354+
}
355+
}
356+
338357
arena_t *
339358
arena_init(tsdn_t *tsdn, unsigned ind, extent_hooks_t *extent_hooks) {
340359
arena_t *arena;
341360

342361
malloc_mutex_lock(tsdn, &arenas_lock);
343362
arena = arena_init_locked(tsdn, ind, extent_hooks);
344363
malloc_mutex_unlock(tsdn, &arenas_lock);
364+
365+
arena_new_create_background_thread(tsdn, ind);
366+
345367
return arena;
346368
}
347369

@@ -475,6 +497,7 @@ arena_choose_hard(tsd_t *tsd, bool internal) {
475497

476498
if (narenas_auto > 1) {
477499
unsigned i, j, choose[2], first_null;
500+
bool is_new_arena[2];
478501

479502
/*
480503
* Determine binding for both non-internal and internal
@@ -486,6 +509,7 @@ arena_choose_hard(tsd_t *tsd, bool internal) {
486509

487510
for (j = 0; j < 2; j++) {
488511
choose[j] = 0;
512+
is_new_arena[j] = false;
489513
}
490514

491515
first_null = narenas_auto;
@@ -545,13 +569,23 @@ arena_choose_hard(tsd_t *tsd, bool internal) {
545569
&arenas_lock);
546570
return NULL;
547571
}
572+
is_new_arena[j] = true;
548573
if (!!j == internal) {
549574
ret = arena;
550575
}
551576
}
552577
arena_bind(tsd, choose[j], !!j);
553578
}
554579
malloc_mutex_unlock(tsd_tsdn(tsd), &arenas_lock);
580+
581+
for (j = 0; j < 2; j++) {
582+
if (is_new_arena[j]) {
583+
assert(choose[j] > 0);
584+
arena_new_create_background_thread(
585+
tsd_tsdn(tsd), choose[j]);
586+
}
587+
}
588+
555589
} else {
556590
ret = arena_get(tsd_tsdn(tsd), 0, false);
557591
arena_bind(tsd, 0, false);

0 commit comments

Comments
 (0)