Skip to content

Commit 0d5e52d

Browse files
committed
Issue #1856: Avoid crashes and lockups when daemon threads run while the
interpreter is shutting down; instead, these threads are now killed when they try to take the GIL.
1 parent e548f5a commit 0d5e52d

File tree

6 files changed

+69
-7
lines changed

6 files changed

+69
-7
lines changed

Include/pythonrun.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,8 @@ PyAPI_FUNC(void) PyByteArray_Fini(void);
214214
PyAPI_FUNC(void) PyFloat_Fini(void);
215215
PyAPI_FUNC(void) PyOS_FiniInterrupts(void);
216216
PyAPI_FUNC(void) _PyGC_Fini(void);
217+
218+
PyAPI_DATA(PyThreadState *) _Py_Finalizing;
217219
#endif
218220

219221
/* Stuff with no proper home (yet) */

Lib/test/test_threading.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import weakref
1313
import os
1414
import subprocess
15+
from test.script_helper import assert_python_ok
1516

1617
from test import lock_tests
1718

@@ -463,7 +464,6 @@ def test_1_join_on_shutdown(self):
463464
"""
464465
self._run_and_join(script)
465466

466-
467467
@unittest.skipUnless(hasattr(os, 'fork'), "needs os.fork()")
468468
def test_2_join_in_forked_process(self):
469469
# Like the test above, but from a forked interpreter
@@ -655,6 +655,49 @@ def my_release_save():
655655
output = "end of worker thread\nend of main thread\n"
656656
self.assertScriptHasOutput(script, output)
657657

658+
def test_6_daemon_threads(self):
659+
# Check that a daemon thread cannot crash the interpreter on shutdown
660+
# by manipulating internal structures that are being disposed of in
661+
# the main thread.
662+
script = """if True:
663+
import os
664+
import random
665+
import sys
666+
import time
667+
import threading
668+
669+
thread_has_run = set()
670+
671+
def random_io():
672+
'''Loop for a while sleeping random tiny amounts and doing some I/O.'''
673+
blank = b'x' * 200
674+
while True:
675+
in_f = open(os.__file__, 'r')
676+
stuff = in_f.read(200)
677+
null_f = open(os.devnull, 'w')
678+
null_f.write(stuff)
679+
time.sleep(random.random() / 1995)
680+
null_f.close()
681+
in_f.close()
682+
thread_has_run.add(threading.current_thread())
683+
684+
def main():
685+
count = 0
686+
for _ in range(40):
687+
new_thread = threading.Thread(target=random_io)
688+
new_thread.daemon = True
689+
new_thread.start()
690+
count += 1
691+
while len(thread_has_run) < count:
692+
time.sleep(0.001)
693+
# Trigger process shutdown
694+
sys.exit(0)
695+
696+
main()
697+
"""
698+
rc, out, err = assert_python_ok('-c', script)
699+
self.assertFalse(err)
700+
658701

659702
class ThreadingExceptionTests(BaseTestCase):
660703
# A RuntimeError should be raised if Thread.start() is called

Misc/NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ What's New in Python 3.2.1?
1010
Core and Builtins
1111
-----------------
1212

13+
- Issue #1856: Avoid crashes and lockups when daemon threads run while the
14+
interpreter is shutting down; instead, these threads are now killed when
15+
they try to take the GIL.
16+
1317
- Issue #9756: When calling a method descriptor or a slot wrapper descriptor,
1418
the check of the object type doesn't read the __class__ attribute anymore.
1519
Fix a crash if a class override its __class__ attribute (e.g. a proxy of the

Python/ceval.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,12 @@ PyEval_RestoreThread(PyThreadState *tstate)
440440
if (gil_created()) {
441441
int err = errno;
442442
take_gil(tstate);
443+
/* _Py_Finalizing is protected by the GIL */
444+
if (_Py_Finalizing && tstate != _Py_Finalizing) {
445+
drop_gil(tstate);
446+
PyThread_exit_thread();
447+
assert(0); /* unreachable */
448+
}
443449
errno = err;
444450
}
445451
#endif

Python/pythonrun.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,8 @@ int Py_IgnoreEnvironmentFlag; /* e.g. PYTHONPATH, PYTHONHOME */
9090
int Py_NoUserSiteDirectory = 0; /* for -s and site.py */
9191
int Py_UnbufferedStdioFlag = 0; /* Unbuffered binary std{in,out,err} */
9292

93+
PyThreadState *_Py_Finalizing = NULL;
94+
9395
/* PyModule_GetWarningsModule is no longer necessary as of 2.6
9496
since _warnings is builtin. This API should not be used. */
9597
PyObject *
@@ -188,6 +190,7 @@ Py_InitializeEx(int install_sigs)
188190
if (initialized)
189191
return;
190192
initialized = 1;
193+
_Py_Finalizing = NULL;
191194

192195
#if defined(HAVE_LANGINFO_H) && defined(HAVE_SETLOCALE)
193196
/* Set up the LC_CTYPE locale, so we can obtain
@@ -388,15 +391,19 @@ Py_Finalize(void)
388391
* the threads created via Threading.
389392
*/
390393
call_py_exitfuncs();
391-
initialized = 0;
392-
393-
/* Flush stdout+stderr */
394-
flush_std_files();
395394

396395
/* Get current thread state and interpreter pointer */
397396
tstate = PyThreadState_GET();
398397
interp = tstate->interp;
399398

399+
/* Remaining threads (e.g. daemon threads) will automatically exit
400+
after taking the GIL (in PyEval_RestoreThread()). */
401+
_Py_Finalizing = tstate;
402+
initialized = 0;
403+
404+
/* Flush stdout+stderr */
405+
flush_std_files();
406+
400407
/* Disable signal handling */
401408
PyOS_FiniInterrupts();
402409

Python/thread_pthread.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -250,9 +250,9 @@ void
250250
PyThread_exit_thread(void)
251251
{
252252
dprintf(("PyThread_exit_thread called\n"));
253-
if (!initialized) {
253+
if (!initialized)
254254
exit(0);
255-
}
255+
pthread_exit(0);
256256
}
257257

258258
#ifdef USE_SEMAPHORES

0 commit comments

Comments
 (0)