[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 14d5331eb5e6c38be12bad421bd59ad0fac9e448 by Victor Stinner in 
branch 'master':
bpo-40234: Revert "bpo-37266: Daemon threads are now denied in subinterpreters 
(GH-14049)" (GH-19456)
https://github.com/python/cpython/commit/14d5331eb5e6c38be12bad421bd59ad0fac9e448


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +18811
pull_request: https://github.com/python/cpython/pull/19456

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Eric Snow]

Eric Snow  added the comment:

I've opened bpo-40234 to address backward incompatibility from this
change (e.g. affecting mod-wsgi).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Ok, I opened issue39812

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

STINNER Victor  added the comment:

> There will be a problem with `concurrent.futures.ProcessPoolExecutor`, which 
> currently launches its management thread as a daemon thread.

Please don't discuss in closed issues.

If you want to support concurrent.futures in subinterpreters, please open a 
separated RFE issue.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Kyle Stanley]

Kyle Stanley  added the comment:

> To me, yes.

If Victor is also on-board with the idea, I can look into writing a patch for 
it (after testing to verify that it allows us to change the daemon threads to 
normal threads in concurrent.futures without issues).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Antoine Pitrou]

Antoine Pitrou  added the comment:

> To me, this seems best implemented as a new public function for the threading 
> module, perhaps something like `threading.register_atexit()`. Would this be 
> reasonable?

To me, yes.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Kyle Stanley]

Kyle Stanley  added the comment:

> The daemon thread itself is not problematic, because ProcessPoolExecutor uses 
> an atexit hook to shutdown itself and therefore join the management thread.

ThreadPoolExecutor also uses an atexit hook for its shutdown process. Also, it 
sets each worker thread to a daemon. So we'd definitely have to address as well 
that prior to killing off daemon threads.

> Perhaps the solution would be to have an additional kind of atexit hooks, 
> which get executed before threads are joined.

Hmm, a potential way to do this might be adding a form of "atexit hook" support 
that's specific to threads. Each registered function would get called in the 
internal `_shutdown()` [1] function in the threading module, just before all of 
the non-daemon threads are joined. To me, this seems best implemented as a new 
public function for the threading module, perhaps something like 
`threading.register_atexit()`. Would this be reasonable?

---

[1] - IIUC, `threading._shutdown()` is called in pylifecycle.c, in 
`wait_for_thread_shutdown()`, which is the way that non-daemon threads are 
joined when the interpreter shuts down in `Py_EndInterpreter()` or is finalized 
in `Py_FinalizeEx()`.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Also cc'ing Kyle for the concurrent.futures issue.

--
nosy: +aeros

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Antoine Pitrou]

Antoine Pitrou  added the comment:

Perhaps the solution would be to have an additional kind of atexit hooks, which 
get executed before threads are joined.

--
nosy: +ncoghlan

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Antoine Pitrou]

Antoine Pitrou  added the comment:

There will be a problem with `concurrent.futures.ProcessPoolExecutor`, which 
currently launches its management thread as a daemon thread.  The daemon thread 
itself is not problematic, because ProcessPoolExecutor uses an atexit hook to 
shutdown itself and therefore join the management thread.

It seems, however, that it's not easy to make the thread non-daemon, because 
atexit hooks are executed *after* non-daemon threads are joined.  That would 
lead to a deadlock: the interpreter would wait for the non-daemon management 
thread to exit, but the ProcessPoolExecutor would wait for the atexit hook to 
be called before telling the management thread to exit.

cc'ing Thomas Moreau, who's worker a lot on this.

--
nosy: +pitrou, tomMoral

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

STINNER Victor  added the comment:

I reported the issue to jep: https://github.com/ninia/jep/issues/229

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

STINNER Victor  added the comment:

FYI python-jep project is broken by this change:
https://bugzilla.redhat.com/show_bug.cgi?id=1792062

"JEP embeds CPython in Java through JNI and is safe to use in a heavily 
threaded environment."
https://github.com/ninia/jep 

FAIL: test_shared_modules_threads (test_shared_modules.TestSharedModules)
--
Traceback (most recent call last):
  File 
"/builddir/build/BUILD/jep-3.9.0/src/test/python/test_shared_modules.py", line 
15, in test_shared_modules_threads
jep_pipe(build_java_process_cmd('jep.test.TestSharedModulesThreads'))
  File "/usr/lib64/python3.9/contextlib.py", line 240, in helper
return _GeneratorContextManager(func, args, kwds)
  File "/usr/lib64/python3.9/contextlib.py", line 83, in __init__
self.gen = func(*args, **kwds)
  File "/builddir/build/BUILD/jep-3.9.0/src/test/python/jep_pipe.py", line 36, 
in jep_pipe
assert False, stderr
AssertionError: b'Exception in thread "main" jep.JepException: : daemon thread are not supported in subinterpreters\n\tat 
/usr/lib64/python3.9/threading.start(threading.py:858)\n\tat 
.(:1)\n\tat jep.Jep.eval(Native Method)\n\tat 
jep.Jep.eval(Jep.java:451)\n\tat 
jep.test.TestSharedModulesThreads.main(TestSharedModulesThreads.java:53)\n'

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

STINNER Victor  added the comment:


New changeset b4e68960b90627422325fdb75f463df1e4153c6e by Victor Stinner in 
branch 'master':
bpo-37266: Add bpo number to the What's New entry (GH614584)
https://github.com/python/cpython/commit/b4e68960b90627422325fdb75f463df1e4153c6e


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

Change by STINNER Victor :


--
pull_requests: +14402
pull_request: https://github.com/python/cpython/pull/14584

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

STINNER Victor  added the comment:

Daemon threads must die. That's a first step towards their death!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

STINNER Victor  added the comment:


New changeset 066e5b1a917ec2134e8997d2cadd815724314252 by Victor Stinner in 
branch 'master':
bpo-37266: Daemon threads are now denied in subinterpreters (GH-14049)
https://github.com/python/cpython/commit/066e5b1a917ec2134e8997d2cadd815724314252


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

Change by STINNER Victor :


--
keywords: +patch
pull_requests: +13911
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/14049

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[Karthikeyan Singaravelan]

Change by Karthikeyan Singaravelan :


--
nosy: +eric.snow

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue37266] Daemon threads must be forbidden in subinterpreters

[STINNER Victor]

New submission from STINNER Victor :

Py_EndInterpreter() calls threading._shutdown() which waits for non-daemon 
threads spawned in the subinterpreters. Problem: daemon threads continue to run 
after threading._shutdown(), but they rely on an interpreter which is being 
finalized and then deleted.

Attached example shows the problem:

$ ./python subinterp_daemon_thread.py 
hello from daemon thread
Fatal Python error: Py_EndInterpreter: not the last thread

Current thread 0x7f13e5926740 (most recent call first):
  File "subinterp_daemon_thread.py", line 23 in 
Aborted (core dumped)

Catching the bug in Py_EndInterpreter() is too late. IMHO we must simply deny 
daemon threads by design in subinterpreters for safety.

In the main interpreter, we provide best effort to prevent crash at exit, but 
IMHO the implementation is ugly :-( ceval.c uses exit_thread_if_finalizing(): 
it immediately exit the current daemon thread if the threads attempts to 
acquire or release the GIL, whereas the interpreter is gone. Problem: we cannot 
release/clear some data structure at Python exit because of that. So 
Py_Finalize() may leak some memory by design, because of daemon threads.

IMHO we can be way stricter in subinterpreters.

I suggest to only modify Python 3.9.

--
components: Interpreter Core
files: subinterp_daemon_thread.py
messages: 345485
nosy: vstinner
priority: normal
severity: normal
status: open
title: Daemon threads must be forbidden in subinterpreters
versions: Python 3.9
Added file: https://bugs.python.org/file48417/subinterp_daemon_thread.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com