Skip to content

BUG: Fix segfault in MT19937 by preventing recursive seed lists - #31775

Merged
charris merged 5 commits into
numpy:mainfrom
Adhyansinghgupta:fix-mt19937-segfault
Jul 3, 2026
Merged

BUG: Fix segfault in MT19937 by preventing recursive seed lists#31775
charris merged 5 commits into
numpy:mainfrom
Adhyansinghgupta:fix-mt19937-segfault

Conversation

@Adhyansinghgupta

@Adhyansinghgupta Adhyansinghgupta commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

PR summary
This PR fixes a Segmentation Fault (stack overflow) that occurs when initialising MT19937 with a recursive or self-referencing list.
Previously, passing a cyclic list (e.g., x = []; x.append(x)) into the generator caused an infinite loop in the Cython unpacking logic, leading to a hard crash.
I added a _has_cycle helper function in bit_generator.pyx to detect self-referencing sequences early. If a cycle is detected, the SeedSequence initialisation now safely raises a ValueError instead of segfaulting.

Closes #28822
First-time committer introduction
Hi everyone! I am a newer contributor looking to dive deep into the Python data science stack. I wanted my first major contribution to be on the systems level rather than just documentation, so hunting down a C/Cython memory bug seemed like a great challenge. I'm excited to learn more from the maintainers here!

AI Disclosure
I used an AI assistant (Google Gemini) strictly as a pair-programming tool to help navigate the Meson/Spin build system and to draft the initial logic for the _has_cycle helper function. I manually compiled the environment, reproduced the bug, and verified the fix locally.

Verification

Here is the local terminal output proving the fix. Instead of a Segmentation Fault, it now safely catches the cycle and throws the expected exception:

Testing MT19937...
Traceback (most recent call last):
  File "/workspaces/numpy/../test_bug.py", line 5, in <module>
    MT19937(x)
  File "numpy/random/_mt19937.pyx", line 131, in numpy.random._mt19937.MT19937.__init__
    BitGenerator.__init__(self, seed)
  File "numpy/random/bit_generator.pyx", line 559, in numpy.random.bit_generator.BitGenerator.__init__
    seed = SeedSequence(seed)
  File "numpy/random/bit_generator.pyx", line 318, in numpy.random.bit_generator.SeedSequence.__init__
    raise ValueError("Seed (entropy) cannot be a recursive or self-referencing list.")
ValueError: Seed (entropy) cannot be a recursive or self-referencing list.

@rkern

rkern commented Jun 26, 2026

Copy link
Copy Markdown
Member

I think the proper fix for this would be to not do a full unguarded recursion in _coerce_to_uint32_array in the case of a sequence. We do not (intend to) accept nested sequences of any kind, just sequences of integers (or strings at that low level, but that was more of an abandoned, undocumented feature rather than a current feature). That way, we can avoid testing for the rare case and just raise an error when we actually encounter the rare case.

We're not in a rush to fix this, so I think it's worth getting right. If you are interested in carrying on with this, I welcome it. Here are a few notes to help prepare a PR for us:

  • Please omit the unhelpful # --- OUR FIX --- comments. They can't be part of the final contribution to us. You may want to look into refining your AI setup so that it doesn't do that. Modern coding agent harnesses don't do that as much as just asking the chat interfaces does.
  • Changes need tests added to the test suite.
  • New functions, even if private, need docstrings (or at least comments) to help readers understand what they are for just as much as public functions do.
  • There are a couple of ways to follow my implementation advice, but probably the best place to do it is inside of _coerce_to_uint32_array just before the recursion. The right exception (and message text) to raise is here.
  • There's a related issue BUG: SeedSequence accepts types outside of spec #27380, and fixing both would make the PR more valuable.

Thank you!

@Adhyansinghgupta

Adhyansinghgupta commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback and clarification , will be looking forward to fixing both related issues within this PR

@Adhyansinghgupta

Adhyansinghgupta commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

I just pushed an update that moves the recursion guard down into the C-layer like you suggested. It now intercepts nested sequences right before the recursion happens and throws the TypeError you linked. I also ripped out that old pure-Python check I had earlier, so the file is much cleaner now.
This should safely handle both the original infinite recursion bug and the invalid types from Issue #27380. I dropped a couple of tests into test_seed_sequence.py to lock it in, and they are passing locally for me.

rkern
rkern reviewed Jun 27, 2026
View reviewed changes
Comment thread numpy/random/bit_generator.pyx Resolved
Comment thread numpy/random/tests/test_seed_sequence.py Resolved
Comment thread numpy/random/tests/test_seed_sequence.py Resolved
@Adhyansinghgupta

Adhyansinghgupta commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Just pushed an update to fix this! I removed the local imports and that stray comment, and also cleaned up a few formatting issues and extra blank lines in the test file.

I was unaware that np.testing already had those assertion wrappers built in, so thanks for the heads-up. Everything is swapped over to use the global assert_raises and assert_raises_regex. Let me know if anything else needs tweaking

@rkern

rkern commented Jun 27, 2026

Copy link
Copy Markdown
Member

The lint check is failing on some formatting issues. Please run the linter before submitting.

https://github.com/numpy/numpy/actions/runs/28299255548/job/83847957504?pr=31775

@rkern rkern closed this Jun 27, 2026
@rkern rkern reopened this Jun 27, 2026
@rkern

rkern commented Jun 27, 2026

Copy link
Copy Markdown
Member

The lint check is failing. Please run the linter before submitting the next fix.

@Adhyansinghgupta

Adhyansinghgupta commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

Sorry for the back-and-forth on the linting. I’m still learning the specific automated check workflow. I've run the linter locally and verified the formatting now. Thanks for your patience.

@Adhyansinghgupta

Adhyansinghgupta commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

All checks have passed, the code has no conflicts with the base branch and is ready to merge, waiting for the maintainer's review.

rkern
rkern approved these changes Jul 2, 2026
View reviewed changes

@rkern rkern left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! LGTM!

@charris
charris merged commit 3de1e7d into numpy:main Jul 3, 2026
87 checks passed
@charris charris mentioned this pull request Jul 4, 2026
Merged
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Jul 4, 2026
charris added a commit that referenced this pull request Jul 4, 2026
@charris
Merge pull request #31857 from charris/backport-31775
4dee265 
BUG: Fix segfault in MT19937 by preventing recursive seed lists (#31775)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rkern rkern rkern approved these changes

Assignees

No one assigned

Labels

00 - Bug component: numpy.random

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BUG: Segfault/stack-overflow from initializing numpy.random._mt19937.MT19937 with recursive list

3 participants

@Adhyansinghgupta @rkern @charris