Fix a background_thread shutdown issue.

1) make sure background thread 0 is always created; and 2) fix synchronization
between thread 0 and the control thread.
This commit is contained in:
Qi Wang 2018-03-30 15:09:05 -07:00 committed by Qi Wang
parent 956c4ad6b5
commit 21eb0d15a6
2 changed files with 29 additions and 21 deletions

View File

@ -380,35 +380,29 @@ background_thread_create_signals_masked(pthread_t *thread,
return create_err; return create_err;
} }
static void static bool
check_background_thread_creation(tsd_t *tsd, unsigned *n_created, check_background_thread_creation(tsd_t *tsd, unsigned *n_created,
bool *created_threads) { bool *created_threads) {
bool ret = false;
if (likely(*n_created == n_background_threads)) { if (likely(*n_created == n_background_threads)) {
return; return ret;
} }
malloc_mutex_unlock(tsd_tsdn(tsd), &background_thread_info[0].mtx); tsdn_t *tsdn = tsd_tsdn(tsd);
label_restart: malloc_mutex_unlock(tsdn, &background_thread_info[0].mtx);
malloc_mutex_lock(tsd_tsdn(tsd), &background_thread_lock);
for (unsigned i = 1; i < ncpus; i++) { for (unsigned i = 1; i < ncpus; i++) {
if (created_threads[i]) { if (created_threads[i]) {
continue; continue;
} }
background_thread_info_t *info = &background_thread_info[i]; background_thread_info_t *info = &background_thread_info[i];
malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); malloc_mutex_lock(tsdn, &info->mtx);
assert(info->state != background_thread_paused); assert(info->state != background_thread_paused);
bool create = (info->state == background_thread_started); bool create = (info->state == background_thread_started);
malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); malloc_mutex_unlock(tsdn, &info->mtx);
if (!create) { if (!create) {
continue; continue;
} }
/*
* To avoid deadlock with prefork handlers (which waits for the
* mutex held here), unlock before calling pthread_create().
*/
malloc_mutex_unlock(tsd_tsdn(tsd), &background_thread_lock);
pre_reentrancy(tsd, NULL); pre_reentrancy(tsd, NULL);
int err = background_thread_create_signals_masked(&info->thread, int err = background_thread_create_signals_masked(&info->thread,
NULL, background_thread_entry, (void *)(uintptr_t)i); NULL, background_thread_entry, (void *)(uintptr_t)i);
@ -424,11 +418,13 @@ label_restart:
abort(); abort();
} }
} }
/* Restart since we unlocked. */ /* Return to restart the loop since we unlocked. */
goto label_restart; ret = true;
break;
} }
malloc_mutex_lock(tsd_tsdn(tsd), &background_thread_info[0].mtx); malloc_mutex_lock(tsdn, &background_thread_info[0].mtx);
malloc_mutex_unlock(tsd_tsdn(tsd), &background_thread_lock);
return ret;
} }
static void static void
@ -446,8 +442,10 @@ background_thread0_work(tsd_t *tsd) {
&background_thread_info[0])) { &background_thread_info[0])) {
continue; continue;
} }
check_background_thread_creation(tsd, &n_created, if (check_background_thread_creation(tsd, &n_created,
(bool *)&created_threads); (bool *)&created_threads)) {
continue;
}
background_work_sleep_once(tsd_tsdn(tsd), background_work_sleep_once(tsd_tsdn(tsd),
&background_thread_info[0], 0); &background_thread_info[0], 0);
} }
@ -464,8 +462,13 @@ background_thread0_work(tsd_t *tsd) {
background_threads_disable_single(tsd, info); background_threads_disable_single(tsd, info);
} else { } else {
malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx); malloc_mutex_lock(tsd_tsdn(tsd), &info->mtx);
/* Clear in case the thread wasn't created. */ if (info->state != background_thread_stopped) {
/* The thread was not created. */
assert(info->state ==
background_thread_started);
n_background_threads--;
info->state = background_thread_stopped; info->state = background_thread_stopped;
}
malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx); malloc_mutex_unlock(tsd_tsdn(tsd), &info->mtx);
} }
} }
@ -593,6 +596,8 @@ background_threads_enable(tsd_t *tsd) {
marked[i] = false; marked[i] = false;
} }
nmarked = 0; nmarked = 0;
/* Thread 0 is required and created at the end. */
marked[0] = true;
/* Mark the threads we need to create for thread 0. */ /* Mark the threads we need to create for thread 0. */
unsigned n = narenas_total_get(); unsigned n = narenas_total_get();
for (i = 1; i < n; i++) { for (i = 1; i < n; i++) {

View File

@ -24,6 +24,9 @@ TEST_BEGIN(test_deferred) {
size_t sz_b = sizeof(bool); size_t sz_b = sizeof(bool);
assert_d_eq(mallctl("background_thread", NULL, NULL, &enable, sz_b), 0, assert_d_eq(mallctl("background_thread", NULL, NULL, &enable, sz_b), 0,
"Failed to enable background threads"); "Failed to enable background threads");
enable = false;
assert_d_eq(mallctl("background_thread", NULL, NULL, &enable, sz_b), 0,
"Failed to disable background threads");
} }
TEST_END TEST_END