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.
  • Loading branch information
d-netto authored May 31, 2024
1 parent 5fc4a60 commit c52eee2
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 @@ -735,7 +735,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 c52eee2

Please sign in to comment.