Skip to content

Commit 9660109

Browse files
dbrattliclaude
andauthored
[Beam] Fix System.Random seeded implementation to use per-instance state (#28)
Rewrote fable_random.erl to use Erlang's functional rand API (seed_s/uniform_s) with per-instance state stored in the process dictionary, keyed by make_ref(). This fixes the bug where creating a second Random(seed) instance would reset the global PRNG state, breaking independent seeded sequences. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent e431d94 commit 9660109

6 files changed

Lines changed: 88 additions & 97 deletions

File tree

src/Fable.Cli/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919
* [All] Fix unnecessary object allocations during AST traversal when visiting `Import` expressions (by Repo Assist)
2020
* [Beam] Fix `System.Random.Next(0)` implementation (by @ncave)
2121
* [Python] Fix `System.Random` seeded implementation (by @ncave)
22+
* [Beam] Fix `System.Random` seeded implementation to use per-instance state (by @dbrattli)
2223
* [Dart] Fix `Array.compareWith` comparing lengths before elements, producing wrong results for arrays with common prefixes (fixes #2961)
2324
* [Python] Fix unsafe option unwrapping in `DateTimeOffset.get_Offset` and regex replacements (by @dbrattli)
2425
* [All] Replace unsafe option `.Value` unwrapping with safe alternatives in Python/Replacements.fs and Rust/Fable2Rust.fs (code scanning alerts IONIDE-006)

src/Fable.Compiler/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919
* [All] Fix unnecessary object allocations during AST traversal when visiting `Import` expressions (by Repo Assist)
2020
* [Beam] Fix `System.Random.Next(0)` implementation (by @ncave)
2121
* [Python] Fix `System.Random` seeded implementation (by @ncave)
22+
* [Beam] Fix `System.Random` seeded implementation to use per-instance state (by @dbrattli)
2223
* [Dart] Fix `Array.compareWith` comparing lengths before elements, producing wrong results for arrays with common prefixes (fixes #2961)
2324
* [Python] Fix unsafe option unwrapping in `DateTimeOffset.get_Offset` and regex replacements (by @dbrattli)
2425
* [All] Replace unsafe option `.Value` unwrapping with safe alternatives in Python/Replacements.fs and Rust/Fable2Rust.fs (code scanning alerts IONIDE-006)

src/Fable.Transforms/Beam/Replacements.fs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4050,18 +4050,24 @@ let private randoms
40504050
match args with
40514051
| [ seed ] -> Helper.LibCall(com, "fable_random", "new_seeded", t, [ seed ], ?loc = r) |> Some
40524052
| _ -> Helper.LibCall(com, "fable_random", "new", t, [], ?loc = r) |> Some
4053-
| "Next", Some _ ->
4053+
| "Next", Some thisArg ->
40544054
match args with
4055-
| [] -> Helper.LibCall(com, "fable_random", "next", t, [], ?loc = r) |> Some
4056-
| [ maxVal ] -> Helper.LibCall(com, "fable_random", "next", t, [ maxVal ], ?loc = r) |> Some
4055+
| [] -> Helper.LibCall(com, "fable_random", "next", t, [ thisArg ], ?loc = r) |> Some
4056+
| [ maxVal ] ->
4057+
Helper.LibCall(com, "fable_random", "next", t, [ thisArg; maxVal ], ?loc = r)
4058+
|> Some
40574059
| [ minVal; maxVal ] ->
4058-
Helper.LibCall(com, "fable_random", "next", t, [ minVal; maxVal ], ?loc = r)
4060+
Helper.LibCall(com, "fable_random", "next", t, [ thisArg; minVal; maxVal ], ?loc = r)
40594061
|> Some
40604062
| _ -> None
4061-
| "NextDouble", Some _ -> Helper.LibCall(com, "fable_random", "next_double", t, [], ?loc = r) |> Some
4062-
| "NextBytes", Some _ ->
4063+
| "NextDouble", Some thisArg ->
4064+
Helper.LibCall(com, "fable_random", "next_double", t, [ thisArg ], ?loc = r)
4065+
|> Some
4066+
| "NextBytes", Some thisArg ->
40634067
match args with
4064-
| [ arr ] -> Helper.LibCall(com, "fable_random", "next_bytes", t, [ arr ], ?loc = r) |> Some
4068+
| [ arr ] ->
4069+
Helper.LibCall(com, "fable_random", "next_bytes", t, [ thisArg; arr ], ?loc = r)
4070+
|> Some
40654071
| _ -> None
40664072
| _ -> None
40674073

src/fable-library-beam/fable_list.erl

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -617,19 +617,19 @@ take(Count, List) ->
617617
%% avoiding any delegation through seq.erl (which would cause circular recursion).
618618
%%
619619
%% Calling convention: Randomizer is fun(ok) -> float() (F# unit -> float curried).
620-
%% Random (System.Random) is the atom ok; all randomness goes through fable_random.
620+
%% Random (System.Random) is a reference() holding per-instance PRNG state.
621621

622622
-spec random_shuffle_by(fun((ok) -> float()), list()) -> list().
623623
random_shuffle_by(Randomizer, Xs) ->
624624
shuffle_loop(Randomizer, erlang:list_to_tuple(Xs), erlang:length(Xs) - 1).
625625

626-
-spec random_shuffle_with(ok, list()) -> list().
627-
random_shuffle_with(_Random, Xs) ->
628-
random_shuffle_by(fun(_) -> fable_random:next_double() end, Xs).
626+
-spec random_shuffle_with(reference(), list()) -> list().
627+
random_shuffle_with(Random, Xs) ->
628+
random_shuffle_by(fun(_) -> fable_random:next_double(Random) end, Xs).
629629

630630
-spec random_shuffle(list()) -> list().
631631
random_shuffle(Xs) ->
632-
random_shuffle_with(ok, Xs).
632+
random_shuffle_with(fable_random:new(), Xs).
633633

634634
-spec random_choice_by(fun((ok) -> float()), list()) -> term().
635635
random_choice_by(_Randomizer, []) ->
@@ -644,13 +644,13 @@ random_choice_by(Randomizer, Xs) ->
644644
lists:nth(erlang:trunc(R * Len) + 1, Xs)
645645
end.
646646

647-
-spec random_choice_with(ok, list()) -> term().
648-
random_choice_with(_Random, Xs) ->
649-
random_choice_by(fun(_) -> fable_random:next_double() end, Xs).
647+
-spec random_choice_with(reference(), list()) -> term().
648+
random_choice_with(Random, Xs) ->
649+
random_choice_by(fun(_) -> fable_random:next_double(Random) end, Xs).
650650

651651
-spec random_choice(list()) -> term().
652652
random_choice(Xs) ->
653-
random_choice_with(ok, Xs).
653+
random_choice_with(fable_random:new(), Xs).
654654

655655
-spec random_choices_by(fun((ok) -> float()), non_neg_integer(), list()) -> list().
656656
random_choices_by(Randomizer, Count, Xs) ->
@@ -678,13 +678,13 @@ choices_loop(Randomizer, Arr, Len, N, Acc) ->
678678
choices_loop(Randomizer, Arr, Len, N - 1, [E | Acc])
679679
end.
680680

681-
-spec random_choices_with(ok, non_neg_integer(), list()) -> list().
682-
random_choices_with(_Random, Count, Xs) ->
683-
random_choices_by(fun(_) -> fable_random:next_double() end, Count, Xs).
681+
-spec random_choices_with(reference(), non_neg_integer(), list()) -> list().
682+
random_choices_with(Random, Count, Xs) ->
683+
random_choices_by(fun(_) -> fable_random:next_double(Random) end, Count, Xs).
684684

685685
-spec random_choices(non_neg_integer(), list()) -> list().
686686
random_choices(Count, Xs) ->
687-
random_choices_with(ok, Count, Xs).
687+
random_choices_with(fable_random:new(), Count, Xs).
688688

689689
-spec random_sample_by(fun((ok) -> float()), non_neg_integer(), list()) -> list().
690690
random_sample_by(Randomizer, Count, Xs) ->
@@ -703,13 +703,13 @@ random_sample_by(Randomizer, Count, Xs) ->
703703
end
704704
end.
705705

706-
-spec random_sample_with(ok, non_neg_integer(), list()) -> list().
707-
random_sample_with(_Random, Count, Xs) ->
708-
random_sample_by(fun(_) -> fable_random:next_double() end, Count, Xs).
706+
-spec random_sample_with(reference(), non_neg_integer(), list()) -> list().
707+
random_sample_with(Random, Count, Xs) ->
708+
random_sample_by(fun(_) -> fable_random:next_double(Random) end, Count, Xs).
709709

710710
-spec random_sample(non_neg_integer(), list()) -> list().
711711
random_sample(Count, Xs) ->
712-
random_sample_with(ok, Count, Xs).
712+
random_sample_with(fable_random:new(), Count, Xs).
713713

714714
%% Partial Fisher-Yates: after Count swaps the first Count positions hold the sample.
715715
sample_loop(_Randomizer, Arr, _Len, Count, I) when I >= Count ->

src/fable-library-beam/fable_random.erl

Lines changed: 50 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -3,62 +3,76 @@
33
-export([
44
new/0,
55
new_seeded/1,
6-
next/0,
76
next/1,
87
next/2,
9-
next_double/0,
10-
next_bytes/1
8+
next/3,
9+
next_double/1,
10+
next_bytes/2
1111
]).
1212

13-
-spec new() -> ok.
14-
-spec new_seeded(integer()) -> ok.
15-
-spec next() -> non_neg_integer().
16-
-spec next(integer()) -> non_neg_integer().
17-
-spec next(integer(), integer()) -> integer().
18-
-spec next_double() -> float().
19-
-spec next_bytes(tuple() | reference()) -> ok.
13+
%% All functions take a Ref (process dictionary key) as the first argument,
14+
%% so each Random instance has its own independent PRNG state.
2015

21-
%% System.Random() — no-op, uses default process-level PRNG state.
16+
%% System.Random() — create a new instance with a random seed.
17+
-spec new() -> reference().
2218
new() ->
23-
ok.
19+
Ref = make_ref(),
20+
put(Ref, rand:seed_s(exsss)),
21+
Ref.
2422

25-
%% System.Random(seed) — seed the process-level PRNG for deterministic output.
23+
%% System.Random(seed) — create a new instance with a deterministic seed.
24+
-spec new_seeded(integer()) -> reference().
2625
new_seeded(Seed) ->
27-
rand:seed(exsss, Seed),
28-
ok.
26+
Ref = make_ref(),
27+
put(Ref, rand:seed_s(exsss, Seed)),
28+
Ref.
2929

3030
%% Random.Next() — returns a non-negative integer less than Int32.MaxValue.
31-
next() ->
32-
rand:uniform(2147483647) - 1.
31+
-spec next(reference()) -> non_neg_integer().
32+
next(Ref) ->
33+
{Val, NewState} = rand:uniform_s(2147483647, get(Ref)),
34+
put(Ref, NewState),
35+
Val - 1.
3336

3437
%% Random.Next(maxValue) — returns a non-negative integer less than maxValue.
35-
next(0) ->
38+
-spec next(reference(), integer()) -> non_neg_integer().
39+
next(_Ref, 0) ->
3640
0;
37-
next(MaxValue) when MaxValue > 0 ->
38-
rand:uniform(MaxValue) - 1;
39-
next(_) ->
41+
next(Ref, MaxValue) when MaxValue > 0 ->
42+
{Val, NewState} = rand:uniform_s(MaxValue, get(Ref)),
43+
put(Ref, NewState),
44+
Val - 1;
45+
next(_, _) ->
4046
erlang:error(badarg).
4147

4248
%% Random.Next(minValue, maxValue) — returns an integer in [minValue, maxValue).
43-
next(MinValue, MaxValue) when MaxValue > MinValue ->
44-
MinValue + rand:uniform(MaxValue - MinValue) - 1;
45-
next(_, _) ->
49+
-spec next(reference(), integer(), integer()) -> integer().
50+
next(Ref, MinValue, MaxValue) when MaxValue > MinValue ->
51+
Range = MaxValue - MinValue,
52+
{Val, NewState} = rand:uniform_s(Range, get(Ref)),
53+
put(Ref, NewState),
54+
MinValue + Val - 1;
55+
next(_, _, _) ->
4656
erlang:error(badarg).
4757

4858
%% Random.NextDouble() — returns a float in [0.0, 1.0).
49-
next_double() ->
50-
rand:uniform().
59+
-spec next_double(reference()) -> float().
60+
next_double(Ref) ->
61+
{Val, NewState} = rand:uniform_s(get(Ref)),
62+
put(Ref, NewState),
63+
Val.
5164

5265
%% Random.NextBytes(byte[]) — fills a byte array with random bytes.
53-
%% Uses the seeded rand state so results are deterministic after seeding.
54-
%% Accepts both direct byte_array tuples and process dictionary references.
55-
next_bytes({byte_array, Size, AtomicsRef}) ->
56-
fill_random_bytes(AtomicsRef, Size, 1);
57-
next_bytes(PdRef) when is_reference(PdRef) ->
58-
next_bytes(get(PdRef)).
66+
-spec next_bytes(reference(), tuple() | reference()) -> ok.
67+
next_bytes(Ref, {byte_array, Size, AtomicsRef}) ->
68+
fill_random_bytes(Ref, AtomicsRef, Size, 1);
69+
next_bytes(Ref, PdRef) when is_reference(PdRef) ->
70+
next_bytes(Ref, get(PdRef)).
5971

60-
fill_random_bytes(_AtomicsRef, 0, _Idx) ->
72+
fill_random_bytes(_Ref, _AtomicsRef, 0, _Idx) ->
6173
ok;
62-
fill_random_bytes(AtomicsRef, Remaining, Idx) ->
63-
atomics:put(AtomicsRef, Idx, rand:uniform(256) - 1),
64-
fill_random_bytes(AtomicsRef, Remaining - 1, Idx + 1).
74+
fill_random_bytes(Ref, AtomicsRef, Remaining, Idx) ->
75+
{Val, NewState} = rand:uniform_s(256, get(Ref)),
76+
put(Ref, NewState),
77+
atomics:put(AtomicsRef, Idx, Val - 1),
78+
fill_random_bytes(Ref, AtomicsRef, Remaining - 1, Idx + 1).

tests/Beam/RandomTests.fs

Lines changed: 6 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -70,33 +70,14 @@ let ``test System.Random works`` () =
7070
let x = rnd.NextDouble()
7171
(x >= 0.0 && x < 1.0) |> equal true
7272

73-
// // Broken test, every new Random seeded instance resets a global seed
74-
// [<Fact>]
75-
// let ``test System.Random seeded works`` () =
76-
// let rnd1 = Random(1234)
77-
// let rnd2 = Random(1234)
78-
// rnd1.Next() |> equal (rnd2.Next())
79-
// rnd1.Next(100) |> equal (rnd2.Next(100))
80-
// rnd1.Next(1000, 10000) |> equal (rnd2.Next(1000, 10000))
81-
// rnd1.NextDouble() |> equal (rnd2.NextDouble())
82-
83-
// Testing the seeded random sequences separately works, but is not ideal
8473
[<Fact>]
8574
let ``test System.Random seeded works`` () =
8675
let rnd1 = Random(1234)
87-
let a1 = rnd1.Next()
88-
let a2 = rnd1.Next(100)
89-
let a3 = rnd1.Next(1000, 10000)
90-
let a4 = rnd1.NextDouble()
9176
let rnd2 = Random(1234)
92-
let b1 = rnd2.Next()
93-
let b2 = rnd2.Next(100)
94-
let b3 = rnd2.Next(1000, 10000)
95-
let b4 = rnd2.NextDouble()
96-
a1 |> equal b1
97-
a2 |> equal b2
98-
a3 |> equal b3
99-
a4 |> equal b4
77+
rnd1.Next() |> equal (rnd2.Next())
78+
rnd1.Next(100) |> equal (rnd2.Next(100))
79+
rnd1.Next(1000, 10000) |> equal (rnd2.Next(1000, 10000))
80+
rnd1.NextDouble() |> equal (rnd2.NextDouble())
10081

10182
[<Fact>]
10283
let ``test System.Random seeded validates arguments`` () =
@@ -124,22 +105,10 @@ let ``test System.Random.NextBytes works`` () =
124105
buffer.Length |> equal 16
125106
buffer = Array.create 16 0uy |> equal false
126107

127-
// // Broken test, every new Random seeded instance resets a global seed
128-
// [<Fact>]
129-
// let ``test System.Random.NextBytes seeded works`` () =
130-
// let buffer1 = Array.create 4 0uy
131-
// let buffer2 = Array.create 4 0uy
132-
// Random(5432).NextBytes(buffer1)
133-
// Random(5432).NextBytes(buffer2)
134-
// buffer1 |> equal buffer2
135-
136-
// Testing the seeded random sequences separately works, but is not ideal
137108
[<Fact>]
138109
let ``test System.Random.NextBytes seeded works`` () =
139110
let buffer1 = Array.create 4 0uy
140111
let buffer2 = Array.create 4 0uy
141-
let rnd1 = Random(5432)
142-
rnd1.NextBytes(buffer1)
143-
let rnd2 = Random(5432)
144-
rnd2.NextBytes(buffer2)
112+
Random(5432).NextBytes(buffer1)
113+
Random(5432).NextBytes(buffer2)
145114
buffer1 |> equal buffer2

0 commit comments

Comments
 (0)