GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-92036: Fix gc_fini_untrack() by vstinner · Pull Request #92037 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Apr 28, 2022

Copy link
Copy Markdown
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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner
gh-92036: Fix gc_fini_untrack()
7acfea9 
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
vstinner requested a review from pablogsal as a code owner April 28, 2022 21:39
@bedevere-bot bedevere-bot added the awaiting core review label Apr 28, 2022
@vstinner vstinner mentioned this pull request Apr 28, 2022
Crash when using sqlite3 with subinterpreters #92036
Closed
@vstinner

vstinner commented Apr 29, 2022

Copy link
Copy Markdown
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:

  • Crash when using sqlite3 with subinterpreters #92036
  • Kodi crashing #90619

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).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@matthuisman matthuisman mentioned this pull request May 1, 2022
Kodi crashes with segfault after changes to the library xbmc/xbmc#21243
Closed
1 of 7 tasks
@comrade-meowski comrade-meowski mentioned this pull request May 3, 2022
Kodi crashing #90619
Closed
@vstinner
vstinner merged commit 1424336 into python:main May 4, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label 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 label May 4, 2022
@miss-islington

miss-islington commented May 4, 2022

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner added the needs backport to 3.9 label May 4, 2022
@miss-islington

miss-islington commented May 4, 2022

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.10 label May 4, 2022
@bedevere-bot

bedevere-bot commented May 4, 2022

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot

bedevere-bot commented May 4, 2022

Copy link
Copy Markdown

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the needs backport to 3.9 label May 4, 2022
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 4, 2022
@vstinner @miss-islington
pythongh-92036: Fix gc_fini_untrack() (pythonGH-92037)
7f99cff 
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
@miss-islington @vstinner
gh-92036: Fix gc_fini_untrack() (GH-92037)
178a238 
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
@miss-islington @vstinner
gh-92036: Fix gc_fini_untrack() (GH-92037)
f84c51e 
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]>
heitbaum added a commit to heitbaum/LibreELEC.tv that referenced this pull request May 17, 2022
@heitbaum
Python3: update to 3.9.13
5c655c7 
release:
- https://www.python.org/downloads/release/python-3913/
changelog:
- https://docs.python.org/release/3.9.13/whatsnew/changelog.html

upstream fix for dropped patch
- python/cpython#80254
- python/cpython#11984
- python/cpython#90228
- python/cpython#92036
- python/cpython#92037
- python/cpython#92297
- bpo-36073
- was
  - python/cpython#30579
  - python/cpython#30580
@heitbaum heitbaum mentioned this pull request May 17, 2022
Python3: update to 3.9.13 LibreELEC/LibreELEC.tv#6494
Merged
hello-adam pushed a commit to hello-adam/cpython that referenced this pull request Jun 2, 2022
@miss-islington @vstinner @hello-adam
pythongh-92036: Fix gc_fini_untrack() (pythonGH-92037)
ede522e 
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

Reviewers

@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@vstinner @miss-islington @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.