Skip to content

Commit 236d2ce

Browse files
d-nettoKristofferC
authored and
KristofferC
committed
ensure we set the right value to gc_first_tid (#54645)
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)
1 parent e52719b commit 236d2ce

File tree

2 files changed

+8
-6
lines changed

2 files changed

+8
-6
lines changed

src/threading.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -710,7 +710,7 @@ void jl_init_threading(void)
710710
jl_atomic_store_release(&jl_all_tls_states, (jl_ptls_t*)calloc(jl_all_tls_states_size, sizeof(jl_ptls_t)));
711711
jl_atomic_store_release(&jl_n_threads, jl_all_tls_states_size);
712712
jl_n_gcthreads = ngcthreads;
713-
gc_first_tid = nthreads;
713+
gc_first_tid = nthreads + nthreadsi;
714714
}
715715

716716
static uv_barrier_t thread_init_done;

test/gc.jl

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ using Test
55
function run_gctest(file)
66
let cmd = `$(Base.julia_cmd()) --depwarn=error --rr-detach --startup-file=no $file`
77
@testset for test_nthreads in (1, 2, 4)
8-
@testset for concurrent_sweep in (0, 1)
9-
new_env = copy(ENV)
10-
new_env["JULIA_NUM_THREADS"] = string(test_nthreads)
11-
new_env["JULIA_NUM_GC_THREADS"] = "$(test_nthreads),$(concurrent_sweep)"
12-
@test success(run(pipeline(setenv(cmd, new_env), stdout = stdout, stderr = stderr)))
8+
@testset for test_nithreads in (0, 1)
9+
@testset for concurrent_sweep in (0, 1)
10+
new_env = copy(ENV)
11+
new_env["JULIA_NUM_THREADS"] = "$test_nthreads,$test_nithreads"
12+
new_env["JULIA_NUM_GC_THREADS"] = "$(test_nthreads),$(concurrent_sweep)"
13+
@test success(run(pipeline(setenv(cmd, new_env), stdout = stdout, stderr = stderr)))
14+
end
1315
end
1416
end
1517
end

0 commit comments

Comments
 (0)