Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-132835: Add defensive NULL checks in mro resolution#134763
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Conversation
Currently, there are a few places where tp_mro could theoreticallybecome NULL, but do not in practice. We should defensively check forNULL values to ensure that any changes do not introduce a crash and thatstate invariants are upheld.
Uh oh!
There was an error while loading.Please reload this page.
bedevere-bot commentedJun 1, 2025
🤖 New build scheduled with the buildbot fleet by@emmatyping for commitc305485 🤖 Results will be shown at: https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F134763%2Fmerge If you want to schedule another build, you need to add the🔨 test-with-buildbots label again. |
The WASI failure is unrelated and should be fixed bypython/buildmaster-config#601 |
picnixz commentedJun 1, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@emmatyping In general, it's better to wait until buildbots are done before committing something new because otherwise we won't see the status of the build on GitHub (it's still visible on the buildbots page but it's harder to find) (usually, it takes around 3-4 hours for the interesting build bots, but the full run can take up to 12 hours) |
@picnixz Oh! Sorry, I seem to have misunderstood how the buildbots work with Github. It's a bit confusing that they only start showing up when they're ready as opposed to when they've been queued, it makes it much harder to know how many to expect unless you know already :/ I generally would add the label and see a number of buildbots show up as active or pending and thought those were the set of buildbots that would be run. I updated the branch when I didn't see any more active buildbots running. I'll definitely be more patient with the builbots going forward. Thank you for letting me know! |
Uh oh!
There was an error while loading.Please reload this page.
Currently, there are a few places where
type->tp_mro
could theoretically be NULL, but are not in practice. We should defensively assert these NULL values don't occur to ensure that any changes do not introduce a crash and that state invariants are upheld.The assertions added in this PR are all instances where a NULL value would get passed to something not expected a NULL, so it is better to catch an assertion failure than crash later on.
There are a few cases where it is OK for the return of
lookup_tp_mro
to be NULL, such as when passed tois_subtype_with_mro
, which handles this explicitly.