-
-
Notifications
You must be signed in to change notification settings - Fork 32.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 our terms of service and privacy 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 theoretically become NULL, but do not in practice. We should defensively check for NULL values to ensure that any changes do not introduce a crash and that state invariants are upheld.
🤖 New build scheduled with the buildbot fleet by @emmatyping for commit c305485 🤖 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 by python/buildmaster-config#601 |
@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! |
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.