From c52eee220654e4b4aca43edb07f5bcf896d3d9bf Mon Sep 17 00:00:00 2001 From: Diogo Netto <61364108+d-netto@users.noreply.github.com> Date: Fri, 31 May 2024 16:51:48 -0300 Subject: [PATCH] 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. --- src/threading.c | 2 +- test/gc.jl | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/threading.c b/src/threading.c index d6e28e3d80828..f10494ad02051 100644 --- a/src/threading.c +++ b/src/threading.c @@ -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; diff --git a/test/gc.jl b/test/gc.jl index d8e6fe07d0240..f924f4952cfb0 100644 --- a/test/gc.jl +++ b/test/gc.jl @@ -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