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

Multithreaded subinterpreters can be running during finalization #126016

Open
ZeroIntensity opened this issue Oct 26, 2024 · 5 comments
Open

Multithreaded subinterpreters can be running during finalization #126016

ZeroIntensity opened this issue Oct 26, 2024 · 5 comments
Assignees
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@ZeroIntensity
Copy link
Member

ZeroIntensity commented Oct 26, 2024

Crash report

What happened?

(Found while investigating a fix for #125920.)

Subinterpreters that are running in a different thread can still be running upon the finalization of the main interpreter, depending on what the thread was doing. Take the following script:

from threading import Thread
import _interpreters


def silly():
    interp = _interpreters.create()
    _interpreters.run_string(interp, "import time; time.sleep(100)")


if __name__ == "__main__":
    Thread(target=silly).start()

On 3.13 and 3.14, killing the process with CTRL+C results in an assertion failure:

python: Python/pylifecycle.c:2480: finalize_subinterpreters: Assertion `!_PyInterpreterState_IsRunningMain(interp)' failed.

However, if the thread is daemon, then the interpreter fully segfaults:

from threading import Thread
import _interpreters


def silly():
    interp = _interpreters.create()
    _interpreters.run_string(interp, "import time; time.sleep(100)")


if __name__ == "__main__":
    Thread(target=silly, daemon=True).start()

I'm going to investigate possible fixes.

cc @ericsnowcurrently

CPython versions tested on:

3.13, CPython main branch

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

No response

Linked PRs

@ZeroIntensity ZeroIntensity added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump topic-subinterpreters 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Oct 26, 2024
@ZeroIntensity ZeroIntensity self-assigned this Oct 26, 2024
@rruuaanng
Copy link
Contributor

Using .join() will not trigger an assertion error. For the reason, please see the PR submitted by Eric and me.

>>> def silly():
...     interp = _interpreters.create()
...     _interpreters.run_string(interp, "import time; time.sleep(100)")
...
>>> if __name__ == "__main__":
...     Thread(target=silly).start()
...
>>>
>>> Thread(target=silly).start()
>>> Thread(target=silly).start()
>>> quit


^CTraceback (most recent call last):
  File "/root/cpython/Lib/threading.py", line 1540, in _shutdown
    _thread_shutdown()
KeyboardInterrupt:
python: Python/pylifecycle.c:2463: finalize_subinterpreters: Assertion `!_PyInterpreterState_IsRunningMain(interp)' failed.
>>> def silly():
...     interp = _interpreters.create()
...     _interpreters.run_string(interp, "import time; time.sleep(100)")
...
>>> t = Thread(target=silly)
>>> t.start()
>>> t.join()
^CTraceback (most recent call last):
  File "<python-input-14>", line 1, in <module>
    t.join()
    ~~~~~~^^
  File "/root/cpython/Lib/threading.py", line 1092, in join
    self._handle.join(timeout)
    ~~~~~~~~~~~~~~~~~^^^^^^^^^
KeyboardInterrupt

@ZeroIntensity
Copy link
Member Author

Yes, I know. The issue is that CTRL+C'ing causes the thread to not finish joining, meaning that the thread state is still marked as running. I'm working on a fix to properly clean up thread states before the interpreter shuts down using pending calls, but daemon threads will probably have to get addressed elsewhere (there's a data race, I think).

@rruuaanng
Copy link
Contributor

rruuaanng commented Oct 27, 2024

If you want, you can insert a signal catcher to catch SIGINT and do error handling on it. And in the error handling iterate over the list of threads and handle them

@ZeroIntensity
Copy link
Member Author

That would result in a thread safety nightmare if the thread was still running :)

@ZeroIntensity
Copy link
Member Author

#126026 addresses non-daemon threads. It's a bit hacky, though, because I have to manually mess with the status of the thread state.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) topic-subinterpreters type-crash A hard crash of the interpreter, possibly with a core dump
Projects
Status: Todo
Development

No branches or pull requests

2 participants