Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-46070: Revert "bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)" #30564

Closed
wants to merge 1 commit into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Jan 12, 2022

This reverts commit 7247407.

Fix a random crash involving subinterpreters on Windows. Revert the
change which made the gc module state per interpreter: the gc module
state is shared again by all interpreters.

https://bugs.python.org/issue36854

…rState (GH-17287)"

This reverts commit 7247407.

Fix a random crash involving subinterpreters on Windows. Revert the
change which made the gc module state per interpreter: the gc module
state is shared again by all interpreters.
@vstinner vstinner changed the title Revert "bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)" bpo-46070: Revert "bpo-36854: Move _PyRuntimeState.gc to PyInterpreterState (GH-17287)" Jan 12, 2022
Copy link
Member

@ericsnowcurrently ericsnowcurrently left a comment

Choose a reason for hiding this comment

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

LGTM

😢

@ericsnowcurrently
Copy link
Member

FTR, this is to resolve https://bugs.python.org/issue46070.

@ericsnowcurrently
Copy link
Member

BTW, do we have any sort of reproducer for which we can add a test? That way we don't cause the same problem when we move GC back to PyInterpreterState later.

@vstinner
Copy link
Member Author

BTW, do we have any sort of reproducer for which we can add a test?

So far, I was only able to reproduce the crash on Windows with Python 3.9 using win_py399_crash_reproducer.py script of https://bugs.python.org/issue46070 I failed to write a reliable reproducer which works on any platform.

I didn't fully understand the root issue. My theory is that an object is shared by two interprereter. The first interpreter runs a GC collection which traverses this shared object, an object finalizer releases the GIL. Now the second interpreter gets the GIL and also runs a GC collection which traverses the same shared object.

The problem is that the PyGC_Head header cannot be modified by two interpreters in parallel: data races are likely and Python does crash somehow.

Since the bug is a crash and so far nobody managed to fully understand it, IMO it's safer to revert the change for now.

@ericsnowcurrently
Copy link
Member

Your theory sounds plausible.

@vstinner
Copy link
Member Author

I wrote a simpler fix: GH-30577.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants