Skip to content

Commit d81f044

Browse files
authored
Clean up atomic access to sleep_check_state (#36785)
Fixes #36699
1 parent b2154d6 commit d81f044

File tree

1 file changed

+12
-14
lines changed

1 file changed

+12
-14
lines changed

src/partr.c

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -330,13 +330,10 @@ static int sleep_check_after_threshold(uint64_t *start_cycles)
330330
static void wake_thread(int16_t tid)
331331
{
332332
jl_ptls_t other = jl_all_tls_states[tid];
333-
if (jl_atomic_load(&other->sleep_check_state) != not_sleeping) {
334-
int16_t state = jl_atomic_exchange(&other->sleep_check_state, not_sleeping); // prohibit it from sleeping
335-
if (state == sleeping) { // see if it was possibly sleeping before now
336-
uv_mutex_lock(&other->sleep_lock);
337-
uv_cond_signal(&other->wake_signal);
338-
uv_mutex_unlock(&other->sleep_lock);
339-
}
333+
if (jl_atomic_bool_compare_exchange(&other->sleep_check_state, sleeping, not_sleeping)) {
334+
uv_mutex_lock(&other->sleep_lock);
335+
uv_cond_signal(&other->wake_signal);
336+
uv_mutex_unlock(&other->sleep_lock);
340337
}
341338
}
342339

@@ -358,7 +355,7 @@ JL_DLLEXPORT void jl_wakeup_thread(int16_t tid)
358355
JULIA_DEBUG_SLEEPWAKE( wakeup_enter = cycleclock() );
359356
if (tid == self || tid == -1) {
360357
// we're already awake, but make sure we'll exit uv_run
361-
if (ptls->sleep_check_state != not_sleeping)
358+
if (jl_atomic_load_relaxed(&ptls->sleep_check_state) == sleeping)
362359
jl_atomic_store(&ptls->sleep_check_state, not_sleeping);
363360
if (uvlock == system_self)
364361
uv_stop(jl_global_event_loop());
@@ -405,7 +402,10 @@ static jl_task_t *get_next_task(jl_value_t *trypoptask, jl_value_t *q)
405402

406403
static int may_sleep(jl_ptls_t ptls)
407404
{
408-
return ptls->sleep_check_state == sleeping;
405+
// sleep_check_state is only transitioned from not_sleeping to sleeping
406+
// by the thread itself. As a result, if this returns false, it will
407+
// continue returning false. If it returns true, there are no guarantees.
408+
return jl_atomic_load_relaxed(&ptls->sleep_check_state) == sleeping;
409409
}
410410

411411
extern volatile unsigned _threadedregion;
@@ -476,8 +476,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q)
476476
JL_UV_UNLOCK();
477477
// optimization: check again first if we may have work to do
478478
if (!may_sleep(ptls)) {
479-
if (ptls->sleep_check_state != not_sleeping)
480-
jl_atomic_store(&ptls->sleep_check_state, not_sleeping); // let other threads know they don't need to wake us
479+
assert(ptls->sleep_check_state == not_sleeping);
481480
start_cycles = 0;
482481
continue;
483482
}
@@ -495,7 +494,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q)
495494
if (!_threadedregion && active && ptls->tid == 0) {
496495
// thread 0 is the only thread permitted to run the event loop
497496
// so it needs to stay alive
498-
if (ptls->sleep_check_state != not_sleeping)
497+
if (jl_atomic_load_relaxed(&ptls->sleep_check_state) != not_sleeping)
499498
jl_atomic_store(&ptls->sleep_check_state, not_sleeping); // let other threads know they don't need to wake us
500499
start_cycles = 0;
501500
continue;
@@ -510,8 +509,7 @@ JL_DLLEXPORT jl_task_t *jl_task_get_next(jl_value_t *trypoptask, jl_value_t *q)
510509
uv_cond_wait(&ptls->wake_signal, &ptls->sleep_lock);
511510
// TODO: help with gc work here, if applicable
512511
}
513-
if (ptls->sleep_check_state != not_sleeping)
514-
jl_atomic_store(&ptls->sleep_check_state, not_sleeping); // let other threads know they don't need to wake us
512+
assert(ptls->sleep_check_state == not_sleeping);
515513
uv_mutex_unlock(&ptls->sleep_lock);
516514
JULIA_DEBUG_SLEEPWAKE( ptls->sleep_leave = cycleclock() );
517515
jl_gc_safe_leave(ptls, gc_state); // contains jl_gc_safepoint

0 commit comments

Comments
 (0)