savestates Device tests abort under a bare cargo test --lib: adopt with_big_stack #25

New Issue

pbwraith · 2026-08-26T13:35:54+02:00

pbwraith commented

2026-08-26 13:35:54 +02:00

!24 (#23) landed two mitigations for the 2 MiB libtest stack abort:

(a) scripts/gate.sh exports RUST_MIN_STACK=8388608 -- immunises THE GATE.
(b) src/test_util.rs::with_big_stack -- runs a test body on an
explicitly sized 8 MiB thread, so a test is independent of the ambient stack
on any runner, with no env var.

!15 (#2) merged the first real Device-constructing tests onto main and
they rely on (a) only. So main now passes its own gate and aborts under the
most natural command a contributor types.

Measured on the !15 merge result (da21ff1), not inferred

Under the documented gate command:

bash scripts/gate.sh
running 8 tests ... test result: ok. 8 passed; 0 failed; finished in 55.94s
GATE_OK   GATE_EXIT=0

Under a bare cargo test --lib with the stack budget at the libtest default
(same shell, same provisioning env from gate.sh, only RUST_MIN_STACK
changed to 2097152):

running 8 tests
test netplay::tests::state_message_constants_match_protocol ... ok
test netplay::tests::ready_ack_send_receive_within_timeout ... ok
test savestates::tests::netplay_process_savestates_uses_in_memory_snapshot ... ok

thread 'savestates::tests::netplay_in_memory_savestate_round_trip_restores_device'
  (149958) has overflowed its stack
fatal runtime error: stack overflow, aborting
error: test failed, to rerun pass `--lib`
Caused by:
  process didn't exit successfully: (signal: 6, SIGABRT: process abort signal)

Note what that costs: the abort kills the whole binary, so the four tests that
had not yet reported are simply gone. One unwrapped test hides every other
result -- which is precisely how #23 stayed invisible while step 3 was
vacuous.

Scope

Wrap the Device-constructing test bodies in savestates with the existing
helper. This is mechanical:

use crate::test_util::with_big_stack;

#[test]
fn netplay_in_memory_savestate_round_trip_restores_device() {
    with_big_stack(|| {
        // existing body, unchanged
    });
}

Apply to every test in src/savestates.rs that constructs or deserialises a
Device -- at minimum
netplay_in_memory_savestate_round_trip_restores_device, and check
netplay_process_savestates_uses_in_memory_snapshot and
offline_explicit_save_stays_on_disk_path_and_never_populates_memory the same
way rather than assuming they are safe because they pass today (they pass
under the gate's 8 MiB, which proves nothing about 2 MiB).

Acceptance

RUST_MIN_STACK=2097152 cargo test --lib runs to completion and reports
8 passed; 0 failed with NO stack overflow / SIGABRT line. Quote the
actual output in the PR.
bash scripts/gate.sh still ends GATE_OK with the same 8 tests.
Do NOT remove RUST_MIN_STACK from scripts/gate.sh -- belt and braces was
a deliberate choice in !24, and it is what protects tests nobody has
remembered to wrap.

Out of scope

Any production change to src/savestates.rs or src/device.rs.
Changing with_big_stack's 8 MiB budget or the MAX_DEVICE_SIZE tripwire.
!11 / !18, which I have already bounced with this same adoption note as
part of their rebases -- do not edit those branches from here.

Verification note for whoever takes this

Do not judge the bare run by the shell exit code. cargo test ... | tail gives
you tail's status, not cargo's -- the SIGABRT is visible in the OUTPUT, not
in $?. Read the text.

`!24` (#23) landed two mitigations for the 2 MiB libtest stack abort: - **(a)** `scripts/gate.sh` exports `RUST_MIN_STACK=8388608` -- immunises THE GATE. - **(b)** `src/test_util.rs::with_big_stack` -- runs a test body on an explicitly sized 8 MiB thread, so a test is independent of the ambient stack on any runner, with no env var. `!15` (#2) merged the first real `Device`-constructing tests onto `main` and they rely on (a) only. So `main` now passes its own gate and aborts under the most natural command a contributor types. ## Measured on the !15 merge result (da21ff1), not inferred Under the documented gate command: bash scripts/gate.sh running 8 tests ... test result: ok. 8 passed; 0 failed; finished in 55.94s GATE_OK GATE_EXIT=0 Under a bare `cargo test --lib` with the stack budget at the libtest default (same shell, same provisioning env from `gate.sh`, only `RUST_MIN_STACK` changed to 2097152): running 8 tests test netplay::tests::state_message_constants_match_protocol ... ok test netplay::tests::ready_ack_send_receive_within_timeout ... ok test savestates::tests::netplay_process_savestates_uses_in_memory_snapshot ... ok thread 'savestates::tests::netplay_in_memory_savestate_round_trip_restores_device' (149958) has overflowed its stack fatal runtime error: stack overflow, aborting error: test failed, to rerun pass `--lib` Caused by: process didn't exit successfully: (signal: 6, SIGABRT: process abort signal) Note what that costs: the abort kills the whole binary, so the four tests that had not yet reported are simply gone. One unwrapped test hides every other result -- which is precisely how #23 stayed invisible while step 3 was vacuous. ## Scope Wrap the `Device`-constructing test bodies in `savestates` with the existing helper. This is mechanical: use crate::test_util::with_big_stack; #[test] fn netplay_in_memory_savestate_round_trip_restores_device() { with_big_stack(|| { // existing body, unchanged }); } Apply to every test in `src/savestates.rs` that constructs or deserialises a `Device` -- at minimum `netplay_in_memory_savestate_round_trip_restores_device`, and check `netplay_process_savestates_uses_in_memory_snapshot` and `offline_explicit_save_stays_on_disk_path_and_never_populates_memory` the same way rather than assuming they are safe because they pass today (they pass under the gate's 8 MiB, which proves nothing about 2 MiB). ## Acceptance - `RUST_MIN_STACK=2097152 cargo test --lib` runs to completion and reports `8 passed; 0 failed` with NO `stack overflow` / SIGABRT line. Quote the actual output in the PR. - `bash scripts/gate.sh` still ends `GATE_OK` with the same 8 tests. - Do NOT remove `RUST_MIN_STACK` from `scripts/gate.sh` -- belt and braces was a deliberate choice in !24, and it is what protects tests nobody has remembered to wrap. ## Out of scope - Any production change to `src/savestates.rs` or `src/device.rs`. - Changing `with_big_stack`'s 8 MiB budget or the `MAX_DEVICE_SIZE` tripwire. - `!11` / `!18`, which I have already bounced with this same adoption note as part of their rebases -- do not edit those branches from here. ## Verification note for whoever takes this Do not judge the bare run by the shell exit code. `cargo test ... | tail` gives you `tail`'s status, not cargo's -- the SIGABRT is visible in the OUTPUT, not in `$?`. Read the text.

pbwraith added the

p2

rust

labels 2026-08-26 13:35:54 +02:00

pbwraith referenced this issue

2026-08-26 13:35:55 +02:00

Allow savestate creation/loading during netplay (#2) #15

pbwraith added the

in-progress

label 2026-08-26 14:01:12 +02:00

pbwraith added the

manual-only