Skip to content

Commit

Permalink
give finalizers their own RNG state
Browse files Browse the repository at this point in the history
fixes #42752
  • Loading branch information
JeffBezanson committed May 6, 2022
1 parent e6fc03d commit a15aa7d
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 6 deletions.
17 changes: 17 additions & 0 deletions src/gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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(jl_task_t *from, uint64_t *to);

void jl_gc_init_finalizer_rng_state(void)
{
jl_rng_split(jl_current_task, &finalizer_rngState[0]);
}

static void run_finalizers(jl_task_t *ct)
{
// Racy fast path:
Expand All @@ -396,9 +405,17 @@ 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));
memcpy(&ct->rngState[0], &finalizer_rngState[0], sizeof(save_rngState));

// This releases the finalizers lock.
jl_gc_run_finalizers_in_list(ct, &copied_list);
arraylist_free(&copied_list);

memcpy(&finalizer_rngState[0], &ct->rngState[0], sizeof(save_rngState));
memcpy(&ct->rngState[0], &save_rngState[0], sizeof(save_rngState));
}

JL_DLLEXPORT void jl_gc_run_pending_finalizers(jl_task_t *ct)
Expand Down
2 changes: 2 additions & 0 deletions src/init.c
Original file line number Diff line number Diff line change
Expand Up @@ -753,6 +753,8 @@ static NOINLINE void _finish_julia_init(JL_IMAGE_SEARCH rel, jl_ptls_t ptls, jl_
JL_GC_POP();
}

jl_gc_init_finalizer_rng_state();

if (jl_options.handle_signals == JL_OPTIONS_HANDLE_SIGNALS_ON)
jl_install_sigint_handler();
}
Expand Down
1 change: 1 addition & 0 deletions src/julia_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ void jl_init_stack_limits(int ismaster, void **stack_hi, void **stack_lo);
jl_task_t *jl_init_root_task(jl_ptls_t ptls, void *stack_lo, void *stack_hi);
void jl_init_serializer(void);
void jl_gc_init(void);
void jl_gc_init_finalizer_rng_state(void);
void jl_init_uv(void);
void jl_init_thread_heap(jl_ptls_t ptls);
void jl_init_int32_int64_cache(void);
Expand Down
12 changes: 6 additions & 6 deletions src/task.c
Original file line number Diff line number Diff line change
Expand Up @@ -730,7 +730,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(jl_task_t *from, uint64_t *to) JL_NOTSAFEPOINT
{
/* TODO: consider a less ad-hoc construction
Ideally we could just use the output of the random stream to seed the initial
Expand All @@ -748,10 +748,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->rngState);
to[1] = 0x5a94851fb48a6e05 * jl_genrandom(from->rngState);
to[2] = 0x3688cf5d48899fa7 * jl_genrandom(from->rngState);
to[3] = 0x867b4bb4c42e5661 * jl_genrandom(from->rngState);
}

JL_DLLEXPORT jl_task_t *jl_new_task(jl_function_t *start, jl_value_t *completion_future, size_t ssize)
Expand Down Expand Up @@ -791,7 +791,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(ct, &t->rngState[0]);
// there is no active exception handler available on this stack yet
t->eh = NULL;
t->sticky = 1;
Expand Down
23 changes: 23 additions & 0 deletions stdlib/Random/test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -995,3 +995,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

0 comments on commit a15aa7d

Please sign in to comment.