Skip to content

Conversation

@vstinner
Copy link
Member

Fix a crash in subinterpreters related to the garbage collector. When
a subinterpreter is deleted, untrack all objects tracked by its GC.
To prevent a crash in deallocator functions expecting objects to be
tracked by the GC, leak a strong reference to these objects on
purpose, so they are never deleted and their deallocator functions
are not called.

Fix a crash in subinterpreters related to the garbage collector. When
a subinterpreter is deleted, untrack all objects tracked by its GC.
To prevent a crash in deallocator functions expecting objects to be
tracked by the GC, leak a strong reference to these objects on
purpose, so they are never deleted and their deallocator functions
are not called.
@vstinner
Copy link
Member Author

@pablogsal @tim-one @pitrou @ericsnowcurrently: Would you mind to review the follow-up of the #30577 fix (1a4d1c1)?

My previous fix was incomplete and Kodi still crash when using the_sqlite3 extension with subintepreters:

Pablo proposed a different approach: "move all these objects into the main interpreter GC state":
#30577

It seems like the root problem is when an object is created in an interpreter A and destroyed in interpreter B, previously, the chain of objects could be broken: _gc_prev and _gc_next could become dangling pointers. My first fix prevents this case. I'm not convinced that moving objects to the main interpreter GC state prevents dangling pointers, but I didn't try to implement this idea.

My plan for sub-interpreters is to ensure that objects never travel between interpreters. The problem is that existing C extensions ("incompatible" with subinterpreters: don't implement PEP 489 multiphase init) can still share mutable containers (like a dict in this case) which exposes indirectly objects from different interpreters.

In this case, the interpreter B deletes an object tracked by the GC of interpreter A, but the interpreter A was deleted and so the object was untracked. Problem: the deallocator function, func_dealloc(), requires the object to be tracked by "a" GC (which one? that's unclear to me).

I'm not really excited by the overall gc_fini_untrack() solution, but for me, it's a practical solution for a practical problem. Fixing all C extensions to prevent leaking Python objects in Py_EndInterpreter() is a very complex problem. For example, it took 15 years to fix https://bugs.python.org/issue1635741 which only fixed a subset of the stdlib (only extensions used at Python startup).

@comrade-meowski comrade-meowski mentioned this pull request May 3, 2022
@vstinner vstinner merged commit 1424336 into python:main May 4, 2022
@vstinner vstinner deleted the gc_fini_untrack branch May 4, 2022 09:59
@vstinner vstinner added the needs backport to 3.10 only security fixes label May 4, 2022
@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label May 4, 2022
@bedevere-bot
Copy link

GH-92296 is a backport of this pull request to the 3.10 branch.

@bedevere-bot
Copy link

GH-92297 is a backport of this pull request to the 3.9 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 4, 2022
Fix a crash in subinterpreters related to the garbage collector. When
a subinterpreter is deleted, untrack all objects tracked by its GC.
To prevent a crash in deallocator functions expecting objects to be
tracked by the GC, leak a strong reference to these objects on
purpose, so they are never deleted and their deallocator functions
are not called.
(cherry picked from commit 1424336)

Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request May 4, 2022
Fix a crash in subinterpreters related to the garbage collector. When
a subinterpreter is deleted, untrack all objects tracked by its GC.
To prevent a crash in deallocator functions expecting objects to be
tracked by the GC, leak a strong reference to these objects on
purpose, so they are never deleted and their deallocator functions
are not called.
(cherry picked from commit 1424336)

Co-authored-by: Victor Stinner <[email protected]>
miss-islington added a commit that referenced this pull request May 4, 2022
Fix a crash in subinterpreters related to the garbage collector. When
a subinterpreter is deleted, untrack all objects tracked by its GC.
To prevent a crash in deallocator functions expecting objects to be
tracked by the GC, leak a strong reference to these objects on
purpose, so they are never deleted and their deallocator functions
are not called.
(cherry picked from commit 1424336)

Co-authored-by: Victor Stinner <[email protected]>
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
Fix a crash in subinterpreters related to the garbage collector. When
a subinterpreter is deleted, untrack all objects tracked by its GC.
To prevent a crash in deallocator functions expecting objects to be
tracked by the GC, leak a strong reference to these objects on
purpose, so they are never deleted and their deallocator functions
are not called.
(cherry picked from commit 1424336)

Co-authored-by: Victor Stinner <[email protected]>
Sign up for free to 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.

3 participants