Skip to content

Commit

Permalink
Take safepoint lock before going to sleep in the scheduler.
Browse files Browse the repository at this point in the history
This avoids a deadlock during exit. Between a thread going to sleep and the thread exiting
  • Loading branch information
gbaraldi committed Nov 4, 2024
1 parent 50713ee commit 26053fb
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,7 @@ void jl_safepoint_end_gc(void);
// The caller should set it **BEFORE** calling this function.
void jl_safepoint_wait_gc(void) JL_NOTSAFEPOINT;
void jl_safepoint_wait_thread_resume(void) JL_NOTSAFEPOINT;

int8_t jl_safepoint_take_sleep_lock(jl_ptls_t ptls) JL_NOTSAFEPOINT_ENTER;
// Set pending sigint and enable the mechanisms to deliver the sigint.
void jl_safepoint_enable_sigint(void);
// If the safepoint is enabled to deliver sigint, disable it
Expand Down
15 changes: 15 additions & 0 deletions src/safepoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,21 @@ void jl_safepoint_wait_thread_resume(void)
jl_atomic_store_release(&ct->ptls->gc_state, state);
uv_mutex_unlock(&ct->ptls->sleep_lock);
}
// This takes the sleep lock and puts the thread in GC_SAFE
int8_t jl_safepoint_take_sleep_lock(jl_ptls_t ptls)
{
int8_t gc_state = jl_gc_safe_enter(ptls);
uv_mutex_lock(&ptls->sleep_lock);
if (jl_atomic_load_relaxed(&ptls->suspend_count)) {
// defer this broadcast until we determine whether uv_cond_wait is really going to be needed
uv_mutex_unlock(&ptls->sleep_lock);
uv_mutex_lock(&safepoint_lock);
uv_cond_broadcast(&safepoint_cond_begin);
uv_mutex_unlock(&safepoint_lock);
uv_mutex_lock(&ptls->sleep_lock);
}
return gc_state;
}

// n.b. suspended threads may still run in the GC or GC safe regions
// but shouldn't be observable, depending on which enum the user picks (only 1 and 2 are typically recommended here)
Expand Down
3 changes: 1 addition & 2 deletions src/scheduler.c
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q,

// the other threads will just wait for an individual wake signal to resume
JULIA_DEBUG_SLEEPWAKE( ptls->sleep_enter = cycleclock() );
int8_t gc_state = jl_gc_safe_enter(ptls);
uv_mutex_lock(&ptls->sleep_lock);
int8_t gc_state = jl_safepoint_take_sleep_lock(ptls); // This puts the thread in GC_SAFE and takes the sleep lock
while (may_sleep(ptls)) {
if (ptls->tid == 0) {
task = wait_empty;
Expand Down

0 comments on commit 26053fb

Please sign in to comment.