gh-137007: Track executor before any possible deallocations #137016
Conversation
Fidget-Spinner
requested review from
brandtbucher
and removed request for
markshannon
| executor->vm_data.warm = true; | ||
| if (_PyJIT_Compile(executor, executor->trace, length)) { | ||
| Py_DECREF(executor); | ||
| return NULL; |
There was a problem hiding this comment.
Note to reviewers: the bad dealloc happens right above this line at line 1233/1238 depending on which diff you view.
|
There's no reproducer because I can't force _PyJIT_Compile to fail except in specific scenarios, or when the GC decides to kick in right when the executor is linked, but not yet tracked. |
|
Unfortunately the patch doesn't fix the issue for me. I double checked that it's applied and still get the abort. Investigating whether the diff necessary to reproduce is the same. |
…/cpython into track-before-deallocating
|
@devdanzin can you please try again? |
|
Still aborting here. Does it stop reproducing for you? |
picnixz
left a comment
picnixz
left a comment
There was a problem hiding this comment.
In the other PR, when allocating the cold executor, there is an error path before making it immortal. Should we track the cold executor before it is immortal and untrack it before making it immortal?
|
@devdanzin does it still reproduce now? |
|
Unfortunately I cannot reproduce the crash even without the patch applied. Tried all known MREs, tinkered with them for over two hours to try to make them repro, and fuzzed with the MREs as seeds. No luck reproducing. |
markshannon
left a comment
markshannon
left a comment
There was a problem hiding this comment.
Looks good.
I'm a bit surprised that the first commit didn't fix this. Did you find out why that was?
|
Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14. |
…thonGH-137016) (cherry picked from commit 97e1901) Co-authored-by: Ken Jin <[email protected]>
|
GH-142541 is a backport of this pull request to the 3.14 branch. |
4 participants
Uh oh!
There was an error while loading. Please reload this page.