Skip to content

Commit

Permalink
ensure we set the right value to gc_first_tid (#54645)
Browse files Browse the repository at this point in the history
This may introduce a correctness issue in the work-stealing termination
loop if we're using interactive threads and GC threads simultaneously.

Indeed, if we forget to add `nthreadsi` to `nthreads`, then we're
checking in the mark-loop termination protocol a range `[gc_first_tid,
gc_first_tid + jl_n_markthreads)` of threads which is "shifted to the
left" compared to what it should be.

This implies that we will not be checking whether the GC threads with
higher TID actually have terminated the mark-loop.

(cherry picked from commit c52eee2)
  • Loading branch information
d-netto authored and KristofferC committed Jun 4, 2024
1 parent 9e2cb49 commit 166a82c
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
2 changes: 1 addition & 1 deletion src/threading.c
Original file line number Diff line number Diff line change
Expand Up @@ -732,7 +732,7 @@ void jl_init_threading(void)
jl_atomic_store_release(&jl_all_tls_states, (jl_ptls_t*)calloc(jl_all_tls_states_size, sizeof(jl_ptls_t)));
jl_atomic_store_release(&jl_n_threads, jl_all_tls_states_size);
jl_n_gcthreads = ngcthreads;
gc_first_tid = nthreads;
gc_first_tid = nthreads + nthreadsi;
}

static uv_barrier_t thread_init_done;
Expand Down
12 changes: 7 additions & 5 deletions test/gc.jl
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@ using Test
function run_gctest(file)
let cmd = `$(Base.julia_cmd()) --depwarn=error --rr-detach --startup-file=no $file`
@testset for test_nthreads in (1, 2, 4)
@testset for concurrent_sweep in (0, 1)
new_env = copy(ENV)
new_env["JULIA_NUM_THREADS"] = string(test_nthreads)
new_env["JULIA_NUM_GC_THREADS"] = "$(test_nthreads),$(concurrent_sweep)"
@test success(run(pipeline(setenv(cmd, new_env), stdout = stdout, stderr = stderr)))
@testset for test_nithreads in (0, 1)
@testset for concurrent_sweep in (0, 1)
new_env = copy(ENV)
new_env["JULIA_NUM_THREADS"] = "$test_nthreads,$test_nithreads"
new_env["JULIA_NUM_GC_THREADS"] = "$(test_nthreads),$(concurrent_sweep)"
@test success(run(pipeline(setenv(cmd, new_env), stdout = stdout, stderr = stderr)))
end
end
end
end
Expand Down

0 comments on commit 166a82c

Please sign in to comment.