[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Sam Gross added the comment: The `pthread_exit` behavior has been a problem for PyTorch and related libraries since Python 3.9. The PyTorch team has tried working around the problems without success (i.e. they keep getting bug reports involving crashes in PyEval_SaveThread/RestoreThread).

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: Yes, I would agree that the new APIs are a useful addition regardless of the PyThread_exit_thread change. As far as the proposed `Py_SetThreadExitCallback` that seems like a fine thing for applications to use, as long as it doesn't impact how

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Gregory P. Smith added the comment: **If** we add a callback API for this, I agree with vstinner's https://bugs.python.org/issue42969#msg402558 commentary on the other callback APIs. We can do this one similarly and keep it simple. - Why? It's the initial simplicity that provides those

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: To be clear, the problem I'm trying to address here is not specific to embedding Python in a C++ application. In fact the issue came to my attention while using Python directly, but loading an extension module that was written in C++ using the

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: Jeremy Maitin-Shepard: "In general, I view hanging threads as the least bad thing to do when faced with either acquiring the GIL or not returning at all. There is a lot of existing usage of Python that currently poses a risk of random crashes and memory

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: Another example where a developer asks to call abort() to notice bugs, whereas Python previously silently ignored it: bpo-36829. Calling abort() is a legit use case, but not really the best default behavior. Again, the problem was solved by letting

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: Gregory P. Smith: Python has many API using callbacks: PEP 445 added PyMem_SetAllocator() to set memory allocator. Adding PyMem_GetAllocator() also made possible to chain allocators and to "hook" into an existing allocator to execute code before and after

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: > In this case that unexpected thread simply disappearing can lead to a > deadlock in our process. This problem also remains me the very complex case of bpo-6721: "Locks in the standard library should be sanitized on fork". The issue title looks simple,

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: In general, I view hanging threads as the least bad thing to do when faced with either acquiring the GIL or not returning at all. There is a lot of existing usage of Python that currently poses a risk of random crashes and memory corruption while

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Gregory P. Smith added the comment: A PR adding a `Py_SetThreadExitCallback(func)` API guaranteeing the callback is called before `pthread_exit()` would allow anyone wanting to deal with their deadlocks to register `abort()` or `while(1) pause();` as their exit callback. I'd be okay with

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: I'm not comfortable with PR 28525 which always hang threads which attempt to acquire the GIL after Python exited. I would prefer to keep the current behavior by default, but give the ability to applications embedding Python to decide what to do. With my

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: I suppose calling `Py_Initialize`, `Py_FinalizeEx`, then `Py_Initialize` again, then `Py_FinalizeEx` again in an embedding application, was already not particularly well supported, since it would leak memory. However, with this change it also leaks

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Gregory P. Smith added the comment: I believe jbms is right that pausing the threads is the only right thing to do when they see tstate_must_exit. The PR is likely correct. -- versions: +Python 3.11, Python 3.9 ___ Python tracker

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Change by Jeremy Maitin-Shepard : -- keywords: +patch pull_requests: +26916 stage: -> patch review pull_request: https://github.com/python/cpython/pull/28525 ___ Python tracker

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: > It looks like the `_thread` module does not actually expose > `PyThread_exit_thread` --- the similarly named `thread_PyThread_exit_thread` > just raises SystemExit. Oh right, I was confused by the function name: "thread_PyThread_exit_thread()". It's a

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: It looks like the `_thread` module does not actually expose `PyThread_exit_thread` --- the similarly named `thread_PyThread_exit_thread` just raises SystemExit. >From a search in the codebase, it appears `PyThread_exit_thread` is currently >used

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: See also bpo-13077 "Windows: Unclear behavior of daemon threads on main thread exit". -- ___ Python tracker ___

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: I don't think that there is a "good default behavior" where Python currently calls PyThread_exit_thread(). IMO we should take the problem from the other side and tries to reduce cases when Python can reach this case. Or even make it impossible if possible.

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: PyThread_exit_thread() is exposed as _thread.exit() and _thread.exit_thread(). PyThread_exit_thread() is only called in take_gil() (at 3 places in the function) if tstate_must_exit(tstate) is true. It happens in two cases: * (by design) at Python exit if a

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: Regarding your suggestion of banning daemon threads: I happened to come across this bug not because of daemon threads but because of threads started by C++ code directly that call into Python APIs. The solution I am planning to implement is to add

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: Regarding your suggestion of adding a hook like `Py_SetThreadExitCallback`, it seems like there are 4 plausible behaviors that such a callback may implement: 1. Abort the process immediately with an error. 2. Exit immediately with the original exit

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: > Change CPython to call abort() instead of pthread_exit() as that situation is > unresolvable and the process dying is better than hanging, partially alive. > That solution isn't friendly, but is better than being silent and allowing > deadlock. A

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Jeremy Maitin-Shepard added the comment: Another possible resolution would to simply make threads that attempt to acquire the GIL after Python starts to finalize hang (i.e. sleep until the process exits). Since the GIL can never be acquired again, this is in some sense the simplest way to

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: See also a discussion about the usefulness of daemon threads: https://github.com/python-trio/trio/issues/2046 I'm more in favor of deprecating daemon threads (in any interpreter, not only in subinterpreters). The current implementation is too fragile. There

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

STINNER Victor added the comment: See also bpo-44434: "_thread module: Remove redundant PyThread_exit_thread() call to avoid glibc fatal error: libgcc_s.so.1 must be installed for pthread_cancel to work". New changeset 45a78f906d2d5fe5381d78466b11763fc56d57ba by Victor Stinner in branch

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Change by Alexey Izbyshev : -- nosy: +izbyshev ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue42969] pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful

Change by Gregory P. Smith : -- title: pthread_exit & PyThread_exit_thread are harmful -> pthread_exit & PyThread_exit_thread from PyEval_RestoreThread etc. are harmful ___ Python tracker