Skip to content

Conversation

@Fidget-Spinner
Copy link
Member

@Fidget-SpinnerFidget-Spinner commented Jul 22, 2025

executor->vm_data.warm= true;
if (_PyJIT_Compile(executor, executor->trace, length)){
Py_DECREF(executor);
returnNULL;
Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Note to reviewers: the bad dealloc happens right above this line at line 1233/1238 depending on which diff you view.

@Fidget-Spinner
Copy link
MemberAuthor

Fidget-Spinner commented Jul 22, 2025

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.

@devdanzin
Copy link
Member

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.

@Fidget-Spinner
Copy link
MemberAuthor

@devdanzin can you please try again?

@devdanzin
Copy link
Member

Still aborting here. Does it stop reproducing for you?

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

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?

@Fidget-Spinner
Copy link
MemberAuthor

@devdanzin does it still reproduce now?

@devdanzin
Copy link
Member

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.

Copy link
Member

@markshannonmarkshannon left a comment

Choose a reason for hiding this comment

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

Looks good.

I'm a bit surprised that the first commit didn't fix this. Did you find out why that was?

@Fidget-SpinnerFidget-Spinner merged commit 97e1901 into python:mainDec 10, 2025
64 checks passed
@Fidget-SpinnerFidget-Spinner added the needs backport to 3.14 bugs and security fixes label Dec 10, 2025
@miss-islington-app
Copy link

Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Dec 10, 2025
…thonGH-137016) (cherry picked from commit 97e1901) Co-authored-by: Ken Jin <kenjin@python.org>
@bedevere-app
Copy link

GH-142541 is a backport of this pull request to the 3.14 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.14 bugs and security fixes label Dec 10, 2025
Fidget-Spinner added a commit that referenced this pull request Dec 10, 2025
…H-137016) (GH-142541) gh-137007: Track executor before any possible deallocations (GH-137016) (cherry picked from commit 97e1901) Co-authored-by: Ken Jin <kenjin@python.org>
Sign up for freeto 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.

4 participants

@Fidget-Spinner@devdanzin@markshannon@picnixz