classification
Title: Doc: subprocess should warn uses on race conditions when multiple threads spawn child processes
Type: behavior Stage:
Components: Documentation Versions: Python 3.9, Python 3.8, Python 3.7, Python 3.6, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: docs@python, eryksun, neologix, owenlin, santagada, sbt, vstinner
Priority: normal Keywords:

Created on 2013-11-27 09:30 by owenlin, last changed 2019-07-24 20:56 by eryksun.

Files
File name Uploaded Description Edit
test.py owenlin, 2013-11-27 09:30
Messages (4)
msg204568 - (view) Author: Owen Lin (owenlin) Date: 2013-11-27 09:30
If we call two subprocess.Popen simultaneously, the second one is blocked until the first one is finished. 

The attached file is a code snippet to reproduce this bug. I can reproduce the bug in version 2.7.3 and 2.7.6 very easily (in few seconds with the code). But it works fine on python3. 


Here is the backtrace of python
==========================================
#0  0x00007f0eba954d2d in read () at ../sysdeps/unix/syscall-template.S:82#1  0x00000000005d8d10 in posix_read (self=0x0, args=(5, 1048576)) at ../Modules/posixmodule.c:6628#2  0x0000000000486896 in PyCFunction_Call (func=<built-in function read>, arg=(5, 1048576), kw=0x0)    at ../Objects/methodobject.c:81#3  0x00000000005278e4 in ext_do_call (func=<built-in function read>, pp_stack=0x7fff1fc0ac80, flags=1, na=0, nk=0)    at ../Python/ceval.c:4331
#4  0x00000000005215cd in PyEval_EvalFrameEx (
    f=Frame 0x298f800, for file /usr/lib/python2.7/subprocess.py, line 478, in _eintr_retry_call (func=<built-in funct
ion read>, args=(5, 1048576)), throwflag=0) at ../Python/ceval.c:2705
#5  0x0000000000523c2e in PyEval_EvalCodeEx (co=0x294b880,     globals={'STDOUT': -2, '_has_poll': True, 'gc': <module at remote 0x29672d0>, 'check_call': <function at remote 0x29c4450>, 'mswindows': False, 'select': <module at remote 0x29676e0>, 'list2cmdline': <function at remote 0x29c45a0>, '__all__': ['Popen', 'PIPE', 'STDOUT', 'call', 'check_call', 'check_output', 'CalledProcessError'], 'errno': <module at remote 0x272d4d8>, '_demo_posix': <function at remote 0x29c4648>, '__package__': None, 'PIPE': -1, '_cleanup': <func
===================================

The fd 5 is actually a pipe. But I cannot find the other end of the pipe. A workaround is using lock around all the Popen()s.
msg204570 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2013-11-27 10:18
The creation of slave_popen is not protected by a lock, and so dummy_thread() may spawn a new process (master) at the same time than the main process (slave). If it occurs at the same time, the master may inherit a pipe of the slave process used internally by the subprocess module to send the exception to the parent process if a Python exception occurs in the child process.

The inheritance of pipes has been partially fixed in Python 3.2 with the new function _posixsubprocess.cloexec_pipe() which uses pipe2() function to atomically create a pipe with the O_CLOEXEC flag set.

The problem has been fixed complelty in Python 3.4 with the PEP 446: all files are now created non inheritable by default, and atomic functions to set the "non inheritable" flag are used when available.

You posted a Python 2.7 script, so I suppose that you are stuck at Python 2. In this case, you can workaround the issue by using a lock around the creation of any subprocess. To fix your example, just surround slave_popen creation with "with lock: ..." or lock.acquire()/lock.release(), as you did for master_popen.

I propose to convert this issue to a documentation issue: we should add a warning explaining that spawning processes in more than one thread can lead to such race condition and hang one or more threads. And suggest to use a lock to workaround such issue.

See also the atfork proposition which includes such lock:
http://bugs.python.org/issue16500

--

Your script has other issues:

- you should pass a file open in write mode for stdout/stderr
- you should close open(os.devnull) files, or open them outside the loop
- you should join the thread, or use a daemon thread. If you don't join threads, you will quickly read the limit of the number of threads
- you can use shell=True to avoid spawning two process to run sleep (or simply use time.sleep :-))
- you need a synchronization between the two threads to ensure that master_popen is created before trying to kill it
msg348384 - (view) Author: Leonardo Santagada (santagada) Date: 2019-07-24 12:30
This is still the case on windows as the pipes created to talk to the process might be inherited by two or more simultaneous CreateProcess calls.

I've found a suggested solution to this:

https://devblogs.microsoft.com/oldnewthing/20111216-00/?p=8873

By only inheriting the stdout/err/in handles and them supporting close_fds for windows.

Would more users be interested in a proper patch for this? For us now we have a lock around Popen.__init__ but that obviously doesn't suport subinterpreters and other calls to CreateProcess that might happen.
msg348406 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2019-07-24 20:56
> This is still the case on windows as the pipes created to talk to the
> process might be inherited by two or more simultaneous CreateProcess 
> calls.

subprocess already uses PROC_THREAD_ATTRIBUTE_HANDLE_LIST to address this problem, at least between its own subprocess.Popen calls. The handles in the list still have to be inheritable, so it does not solve the problem with os.system and os.spawn* calls that are concurrent with subprocess.Popen -- nor extension-module, ctypes, cffi, or PyWin32 code  in the wild that inherits handles without PROC_THREAD_ATTRIBUTE_HANDLE_LIST. There's a warning about this in the docs:

https://docs.python.org/3/library/subprocess.html#subprocess.STARTUPINFO.lpAttributeList

It's why we can't use the handle list to implement pass_fds in Windows and why the general capability is buried in STARTUPINFO, instead of being exposed as a high-level Popen parameter.
History
Date User Action Args
2019-07-24 20:56:43eryksunsetnosy: + eryksun
messages: + msg348406
2019-07-24 12:30:27santagadasetnosy: + santagada

messages: + msg348384
versions: + Python 3.6, Python 3.7, Python 3.8, Python 3.9
2013-11-27 10:18:41vstinnersetassignee: docs@python

nosy: + docs@python
components: + Documentation
title: Python get stuck in second Popen call -> Doc: subprocess should warn uses on race conditions when multiple threads spawn child processes
2013-11-27 10:18:06vstinnersetnosy: + vstinner, neologix, sbt
messages: + msg204570
2013-11-27 09:30:46owenlincreate