Skip to content

Commit c52eee2

Browse files
authored
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.
1 parent 5fc4a60 commit c52eee2

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
@@ -735,7 +735,7 @@ void jl_init_threading(void)
735735
jl_atomic_store_release(&jl_all_tls_states, (jl_ptls_t*)calloc(jl_all_tls_states_size, sizeof(jl_ptls_t)));
736736
jl_atomic_store_release(&jl_n_threads, jl_all_tls_states_size);
737737
jl_n_gcthreads = ngcthreads;
738-
gc_first_tid = nthreads;
738+
gc_first_tid = nthreads + nthreadsi;
739739
}
740740

741741
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)