From 85b895bb6919ea86d9b6201108af3a66930a349b Mon Sep 17 00:00:00 2001 From: Jeff Bezanson Date: Thu, 14 Jul 2022 15:50:29 -0400 Subject: [PATCH] give finalizers their own RNG state (#45212) fixes #42752 --- src/gc.c | 16 ++++++++++++++++ src/task.c | 12 ++++++------ stdlib/Random/src/RNGs.jl | 1 + stdlib/Random/test/runtests.jl | 23 +++++++++++++++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/gc.c b/src/gc.c index 3d120cf47cccd..c45ff8206ca67 100644 --- a/src/gc.c +++ b/src/gc.c @@ -375,6 +375,15 @@ static void jl_gc_run_finalizers_in_list(jl_task_t *ct, arraylist_t *list) ct->sticky = sticky; } +static uint64_t finalizer_rngState[4]; + +void jl_rng_split(uint64_t to[4], uint64_t from[4]); + +JL_DLLEXPORT void jl_gc_init_finalizer_rng_state(void) +{ + jl_rng_split(finalizer_rngState, jl_current_task->rngState); +} + static void run_finalizers(jl_task_t *ct) { // Racy fast path: @@ -396,9 +405,16 @@ static void run_finalizers(jl_task_t *ct) } jl_atomic_store_relaxed(&jl_gc_have_pending_finalizers, 0); arraylist_new(&to_finalize, 0); + + uint64_t save_rngState[4]; + memcpy(&save_rngState[0], &ct->rngState[0], sizeof(save_rngState)); + jl_rng_split(ct->rngState, finalizer_rngState); + // This releases the finalizers lock. jl_gc_run_finalizers_in_list(ct, &copied_list); arraylist_free(&copied_list); + + memcpy(&ct->rngState[0], &save_rngState[0], sizeof(save_rngState)); } JL_DLLEXPORT void jl_gc_run_pending_finalizers(jl_task_t *ct) diff --git a/src/task.c b/src/task.c index c12cb5a522099..22a5ad214e0b8 100644 --- a/src/task.c +++ b/src/task.c @@ -729,7 +729,7 @@ uint64_t jl_genrandom(uint64_t rngState[4]) JL_NOTSAFEPOINT return res; } -static void rng_split(jl_task_t *from, jl_task_t *to) JL_NOTSAFEPOINT +void jl_rng_split(uint64_t to[4], uint64_t from[4]) JL_NOTSAFEPOINT { /* TODO: consider a less ad-hoc construction Ideally we could just use the output of the random stream to seed the initial @@ -747,10 +747,10 @@ static void rng_split(jl_task_t *from, jl_task_t *to) JL_NOTSAFEPOINT 0x3688cf5d48899fa7 == hash(UInt(3))|0x01 0x867b4bb4c42e5661 == hash(UInt(4))|0x01 */ - to->rngState[0] = 0x02011ce34bce797f * jl_genrandom(from->rngState); - to->rngState[1] = 0x5a94851fb48a6e05 * jl_genrandom(from->rngState); - to->rngState[2] = 0x3688cf5d48899fa7 * jl_genrandom(from->rngState); - to->rngState[3] = 0x867b4bb4c42e5661 * jl_genrandom(from->rngState); + to[0] = 0x02011ce34bce797f * jl_genrandom(from); + to[1] = 0x5a94851fb48a6e05 * jl_genrandom(from); + to[2] = 0x3688cf5d48899fa7 * jl_genrandom(from); + to[3] = 0x867b4bb4c42e5661 * jl_genrandom(from); } JL_DLLEXPORT jl_task_t *jl_new_task(jl_function_t *start, jl_value_t *completion_future, size_t ssize) @@ -790,7 +790,7 @@ JL_DLLEXPORT jl_task_t *jl_new_task(jl_function_t *start, jl_value_t *completion // Inherit logger state from parent task t->logstate = ct->logstate; // Fork task-local random state from parent - rng_split(ct, t); + jl_rng_split(t->rngState, ct->rngState); // there is no active exception handler available on this stack yet t->eh = NULL; t->sticky = 1; diff --git a/stdlib/Random/src/RNGs.jl b/stdlib/Random/src/RNGs.jl index 115034d3e3988..f79f113bc95eb 100644 --- a/stdlib/Random/src/RNGs.jl +++ b/stdlib/Random/src/RNGs.jl @@ -388,6 +388,7 @@ end function __init__() seed!(GLOBAL_RNG) + ccall(:jl_gc_init_finalizer_rng_state, Cvoid, ()) end diff --git a/stdlib/Random/test/runtests.jl b/stdlib/Random/test/runtests.jl index 616aa80a20dca..b7c6cb10b197d 100644 --- a/stdlib/Random/test/runtests.jl +++ b/stdlib/Random/test/runtests.jl @@ -994,3 +994,26 @@ end @test minimum(m) >= 0.094 @test maximum(m) <= 0.106 end + +# issue #42752 +# test that running finalizers that launch tasks doesn't change RNG stream +function f42752(do_gc::Bool, cell = (()->Any[[]])()) + a = rand() + if do_gc + finalizer(cell[1]) do _ + @async nothing + end + cell[1] = nothing + GC.gc() + end + b = rand() + (a, b) +end +guardseed() do + for _ in 1:4 + Random.seed!(1) + val = f42752(false) + Random.seed!(1) + @test f42752(true) === val + end +end