gh-142476: fix memory leak when creating JIT executors#142492
Conversation
|
I believe a news entry is not required for this change. |
Sorry, something went wrong.
Fidget-Spinner
left a comment
There was a problem hiding this comment.
Wow, great catch!
Could you please add a comment(review below) so that we know why this is needed?
Sorry, something went wrong.
|
The failing Windows test is likely because of https://github.com/python/cpython/pull/137016/files. Could you please try pulling in the changes from there? |
Sorry, something went wrong.
|
Yes, applying the changes from #137016 fixes the Windows crash. |
Sorry, something went wrong.
|
We need this to be merged first https://github.com/python/cpython/pull/137016/files |
Sorry, something went wrong.
|
Also, I'm not sure I get why we DECREF Maybe @markshannon can shed a light to this, because of #118459 |
Sorry, something went wrong.
Detach is for removing the executor from a code object. The code object owns a strong reference to the executor so we must decref it. |
Sorry, something went wrong.
|
The crashes were caused by a double untrack exposed by the fix.
The fix: Guard the untrack call with |
Sorry, something went wrong.
|
So I'm gonna leave this around for a few more days. However could you please add a code comment above the if that tp_traverse might untrack the onject once it breaks cycles, so we need to not double-untrack it please? |
Sorry, something went wrong.
|
Got it, sounds good — I’ll do that. |
Sorry, something went wrong.
|
@sergey-miryanov The project doesn't even build right now, so I'm unable to verify it. I'm building Python from the Build Logs
|
Sorry, something went wrong.
|
@ashm-dev does the project build if you apply this patch here to main? |
Sorry, something went wrong.
|
@Fidget-Spinner No, it doesn't build even with my patch applied. It looks like the JIT is broken again. |
Sorry, something went wrong.
Fidget-Spinner
left a comment
There was a problem hiding this comment.
It's not broken (apart from the refcount leak I think), the current patch seems to be breaking it.
Sorry, something went wrong.
|
But it doesn't build on main either, even without my patch. I posted the logs above. |
Sorry, something went wrong.
sergey-miryanov
left a comment
There was a problem hiding this comment.
Looks good to me. Thanks!
Sorry, something went wrong.
@ashm-dev I think you're misunderstanding me. I'm asking if it builds with this patch currently on this branch? |
Sorry, something went wrong.
e2a7db7
into
python:main
Dec 19, 2025
|
I'm seeing GHA segfaults in x86_64-pc-windows-msvc/msvc (Debug) on this commit and the subsequent one, but not on earlier commits: |
Sorry, something went wrong.
edited by bedevere-app
Bot
LoadingUh oh!
There was an error while loading. Please reload this page.
Copy link Copy MarkdownSorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.