Skip to content

test_broken_bignum: stop using fork + waitpid#962

Merged
byroot merged 2 commits intoruby:masterfrom
k0kubun:test-broken-bignum
Mar 26, 2026
Merged

test_broken_bignum: stop using fork + waitpid#962
byroot merged 2 commits intoruby:masterfrom
k0kubun:test-broken-bignum

Conversation

@k0kubun
Copy link
Member

@k0kubun k0kubun commented Mar 25, 2026

The raw fork + Process.waitpid2 pattern causes a SEGV on i686-linux in ruby/ruby CI. The crash occurs in waitpid_blocking_no_SIGCHLD going through the 32-bit vdso, which is a Ruby VM / kernel interaction issue unrelated to the json gem itself.

example failure: https://github.com/ruby/ruby/actions/runs/23563393907/job/68608786831

@k0kubun k0kubun force-pushed the test-broken-bignum branch 2 times, most recently from ab1cd66 to 03bbab2 Compare March 25, 2026 23:20
@k0kubun k0kubun marked this pull request as ready for review March 25, 2026 23:30
@byroot
Copy link
Member

byroot commented Mar 26, 2026

I don't see assert_separately being used in your patch?

@byroot
Copy link
Member

byroot commented Mar 26, 2026

Ah, I remember now, assert_separately had issues: 1f5e849

The raw fork + Process.waitpid2 pattern causes a SEGV on i686-linux in
ruby/ruby CI. The crash occurs in waitpid_blocking_no_SIGCHLD going
through the 32-bit vdso, which is a Ruby VM / kernel interaction issue
unrelated to the json gem itself.

Replace the fork-based approach with an in-process test that temporarily
patches Integer#to_s and restores it via ensure. This avoids the fork
SEGV on i686, works under valgrind (assert_separately doesn't), and
is guarded to only run on CRuby (not JRuby/TruffleRuby) where the
C extension behavior being tested exists.
@byroot byroot force-pushed the test-broken-bignum branch 2 times, most recently from ec64ecb to 4ae952a Compare March 26, 2026 06:12
@k0kubun
Copy link
Member Author

k0kubun commented Mar 26, 2026

Sorry, I changed the patch after failing JRuby and TruffleRuby jobs, so the PR description was outdated. Thank you for taking care of this 🙏

@byroot
Copy link
Member

byroot commented Mar 26, 2026

Welcome, thanks for finding and reporting that problem ❤️

@byroot byroot merged commit d0b47b0 into ruby:master Mar 26, 2026
41 checks passed
@byroot byroot changed the title test_broken_bignum: use assert_separately instead of fork + waitpid2 test_broken_bignum: stop using fork + waitpid Mar 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants