Skip to content

Commit

Permalink
Change loads of jl_gc_disable_counter to use acquire semantics.
Browse files Browse the repository at this point in the history
Stores already use seq_cst.
  • Loading branch information
maleadt committed May 24, 2023
1 parent e4559c9 commit 248fa30
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 5 deletions.
4 changes: 2 additions & 2 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -3497,7 +3497,7 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection)

jl_task_t *ct = jl_current_task;
jl_ptls_t ptls = ct->ptls;
if (jl_atomic_load_relaxed(&jl_gc_disable_counter)) {
if (jl_atomic_load_acquire(&jl_gc_disable_counter)) {
size_t localbytes = jl_atomic_load_relaxed(&ptls->gc_num.allocd) + gc_num.interval;
jl_atomic_store_relaxed(&ptls->gc_num.allocd, -(int64_t)gc_num.interval);
static_assert(sizeof(_Atomic(uint64_t)) == sizeof(gc_num.deferred_alloc), "");
Expand Down Expand Up @@ -3548,7 +3548,7 @@ JL_DLLEXPORT void jl_gc_collect(jl_gc_collection_t collection)
gc_invoke_callbacks(jl_gc_cb_pre_gc_t,
gc_cblist_pre_gc, (collection));

if (!jl_atomic_load_relaxed(&jl_gc_disable_counter)) {
if (!jl_atomic_load_acquire(&jl_gc_disable_counter)) {
JL_LOCK_NOGC(&finalizers_lock); // all the other threads are stopped, so this does not make sense, right? otherwise, failing that, this seems like plausibly a deadlock
#ifndef __clang_gcanalyzer__
if (_jl_gc_collect(ptls, collection)) {
Expand Down
2 changes: 1 addition & 1 deletion src/safepoint.c
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ int jl_safepoint_start_gc(void)
// Foreign thread adoption disables the GC and waits for it to finish, however, that may
// introduce a race between it and this thread checking if the GC is enabled and only
// then setting jl_gc_running. To avoid that, check again now that we won that race.
if (jl_atomic_load_relaxed(&jl_gc_disable_counter)) {
if (jl_atomic_load_acquire(&jl_gc_disable_counter)) {
uv_mutex_unlock(&safepoint_lock);
return 0;
}
Expand Down
4 changes: 2 additions & 2 deletions src/threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,11 @@ JL_DLLEXPORT jl_gcframe_t **jl_adopt_thread(void) JL_NOTSAFEPOINT_LEAVE
// we can't use a normal safepoint because we don't have signal handlers yet.
// we also can't use jl_safepoint_wait_gc because that assumes we're in a task.
jl_atomic_fetch_add(&jl_gc_disable_counter, 1);
while (jl_atomic_load_relaxed(&jl_gc_running) || jl_atomic_load_acquire(&jl_gc_running)) {
while (jl_atomic_load_acquire(&jl_gc_running)) {
jl_cpu_pause();
}
// In `jl_safepoint_start_gc` we observe if a foreign thread has asked to disable the GC
// Guaranteeing the order of events.
// Guaranteeing the order of events.

// initialize this thread (assign tid, create heap, set up root task)
jl_ptls_t ptls = jl_init_threadtls(-1);
Expand Down

0 comments on commit 248fa30

Please sign in to comment.