Function-level import in os triggering an threaded import deadlock

[michaeltsai]

BPO	1590864
Nosy	@Yhg1s, @brettcannon, @gpshead, @ronaldoussoren, @ericsnowcurrently
Files	import_lock_fork_deadlock.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/Yhg1s'
closed_at = <Date 2009-09-16.20:04:06.243>
created_at = <Date 2006-11-05.16:06:24.000>
labels = ['type-bug', 'library']
title = 'Function-level import in os triggering an threaded import deadlock'
updated_at = <Date 2012-04-14.05:54:28.958>
user = 'https://bugs.python.org/michaeltsai'

bugs.python.org fields:

activity = <Date 2012-04-14.05:54:28.958>
actor = 'eric.snow'
assignee = 'twouters'
closed = True
closed_date = <Date 2009-09-16.20:04:06.243>
closer = 'twouters'
components = ['Library (Lib)']
creation = <Date 2006-11-05.16:06:24.000>
creator = 'michaeltsai'
dependencies = []
files = ['14553']
hgrepos = []
issue_num = 1590864
keywords = ['patch']
message_count = 14.0
messages = ['30451', '30452', '30453', '30454', '30455', '30456', '85068', '85190', '85211', '86234', '88891', '90866', '92716', '138130']
nosy_count = 10.0
nosy_names = ['twouters', 'brett.cannon', 'gregory.p.smith', 'astrand', 'ronaldoussoren', 'michaeltsai', 'kosuha', 'abaron', 'eric.snow', 'Bryan.Schmersal']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'test needed'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue1590864'
versions = ['Python 2.6']

[michaeltsai]

When I use subprocess.py from a child thread, sometimes it deadlocks. I
determined that the new process is blocked during an import:

#0 0x90024427 in semaphore_wait_signal_trap ()
#1 0x90028414 in pthread_cond_wait ()
#2 0x004c77bf in PyThread_acquire_lock (lock=0x3189a0, waitflag=1)
at Python/thread_pthread.h:452
#3 0x004ae2a6 in lock_import () at Python/import.c:266
#4 0x004b24be in PyImport_ImportModuleLevel (name=0xaad74 "errno",
globals=0xbaed0, locals=0x502aa0, fromlist=0xc1378, level=-1) at
Python/import.c:2054
#5 0x0048d2e2 in builtin___import__ (self=0x0, args=0x53724c90,
kwds=0x0) at Python/bltinmodule.c:47
#6 0x0040decb in PyObject_Call (func=0xa94b8, arg=0x53724c90,
kw=0x0) at Objects/abstract.c:1860

and that the code in question is in os.py:

def _execvpe(file, args, env=None):
     from errno import ENOENT, ENOTDIR

I think the problem is that since exec (the C function) hasn't yet been
called in the new process, it's inherited from the fork a lock that's already
held. The main process will eventually release its copy of the lock, but this
will not unlock it in the new process, so it deadlocks.

If I change os.py so that it imports the constants outside of
_execvpe, the new process no longer blocks in this way.

This is on Mac OS X 10.4.8.

[astrand]

Can you provide a test case or sample code that demonstrates this problem?

I'm a bit unsure of if this really is a subprocess bug or a more general Python bug.

[michaeltsai]

I don't have time at the moment to write sample code that reproduces this. But, FYI, I was using PyObjC to create the threads. It might not happen with "threading" threads. And second, I think it's a bug in os.py, not in subprocess.py. Sorry for the confusion.

[astrand]

Since both the reporter and I believes that this is not a bug in the subprocess module, I'm stepping back.

[kosuha]

I confirm that problem with deadlock on execution of PyImport_ImportModuleLevel exists. Here is a working example:

1. Python was embbeded with C++ application using Python for scripting support:

------------------------------------------------------------------
PyThreadState * InitThreadScripting()
{
ASSERT_KOBOLD( g_pyMainThreadState );

	// get the global lock
	PyEval_AcquireLock();
	// get a reference to the PyInterpreterState
	PyInterpreterState * mainInterpreterState = g_pyMainThreadState->interp;
	ASSERT_KOBOLD( mainInterpreterState );
	// create a thread state object for this thread
	PyThreadState * myThreadState = PyThreadState_New(mainInterpreterState);
	// free the lock
	PyEval_ReleaseLock();

	return myThreadState;
}

---------------------------------------------------------------------
void DeadLock()
{
Py_Initialize();
PyEval_InitThreads();
g_AiGlobals     = new Py::Dict();
g_pyInterpState = PyInterpreterState_New();

// save a pointer to the main PyThreadState object
g_pyMainThreadState = PyThreadState_Get();
ASSERT_KOBOLD( g_pyMainThreadState );

// release the lock
PyEval_ReleaseLock();

g_pyWorldThreadState = InitThreadScripting();

// import sys
// sys.path.append ("./scripts")
// 
PyObject	*p = PyImport_ImportModuleEx ("sys", **g_AiGlobals, NULL, NULL);
Py::Module	mod_sys (p);
Py::List	path = mod_sys.getAttr ("path");
path.append (Py::String ("scripts"));
path.append (Py::String ("scripts/sys"));
path.append (Py::String ("../../scripts"));
path.append (Py::String ("../../scripts/sys"));
Py_XDECREF (p);

// HERE IT OCCURS //
Log.ScriptsSrc("Python", "Running startup python scripts...");
PyObject *p = PyImport_ImportModuleEx ("startup", **g_AiGlobals, NULL, NULL); // <<< Here
if (reload) PyImport_ReloadModule (p);
Py::Module	module (p);
Py_XDECREF (p);
}

Execution locks right on PyImport_ImportModuleEx.

Code from sturtup.py:
------------------------------------------------------------------------------------
# This module is sturtup script.
# Here we are redirecting output and checking for server version.
################################################################################

import sys
from consts import *   # Import of constants
import config as cfg   # Import of configuration constants
reload(cfg)  

################################################################################

# ----------------------------------------------------------------------------------------------------------------------------------------------------------------

class OurLogStdErr:
    def write (self, txt):
        printLog (txt, PRINT_ERROR)
        
# ----------------------------------------------------------------------------------------------------------------------------------------------------------------
class OurLogStdOut:
    def write (self, txt):
        printLog (txt)

# ----------------------------------------------------------------------------------------------------------------------------------------------------------------

def CheckServerVersion():
    # Checking for server build
    if GetServerBuild() < MIN_SERVER_BUILD:
        printLog( "YOU ARE TRYING TO RUN PYTHON SCRIPTS ON OUTDATED SERVER BUILD!\
\nREQUIRED SERVER BUILD: %s\
\nPlease Update your server core before running server!\
\nScripting Engine will be Shut Down!"\
        % (MIN_SERVER_BUILD), PRINT_ERROR )

        killScripting()
        
# ----------------------------------------------------------------------------------------------------------------------------------------------------------------
def GetScriptsVersion():
	return SCRIPTS_VERSION

################################################################################
# Startup code here:

# Redirecting errors:
sys.stderr = OurLogStdErr()

# Redirecting output:
sys.stdout = OurLogStdOut()

---

[ronaldoussoren]

Do you have sample code that reproduces this problem? (Not necessarily code that has this problem 100% of the time)

[brettcannon]

It seems the import lock is being triggered because of an import being
made in a function call by something else being imported. And because it
is the os module one can't pull out the function-level imports without
causing problems.

Closing as "won't fix".

[ronaldoussoren]

I don't understand why the function-level imports cannot be removed.

Wouldn't it be possible to do something like this:

from errno import ENOENT as _ENOENT, ENOTDIR as _ENOTDIR
def _execvpe(file, args, env=None):
   pass # Use _ENOENT and _ENOTDIR in this code

BTW. it is IMO rather strange to close issues as wont fix when there is a
real error in supported and not deprecated code.

[brettcannon]

First, because os is such a common module that hiding some uncommon
imports at the module level helps with startup costs.

Second, this is not a bug as the code is not behaving in an improper
manner. The import lock is doing what it is supposed to be doing and
importing at the module level is still okay.

You can open the issue again if you want, but I think this is not worth
changing.

[abaron]

Hi,
We have the same problem while running two threads where one is running
a subprocess command and the other is importing modules. This will
cause a deadlock and this IS a bug!!!
This happens quite often on a slow machine (once every 2-3 runs).

[brettcannon]

Been thinking about it and as a compromise to people who view this as a
bug I am re-opening it but lowering the priority.

[Yhg1s]

Here's a preliminary fix (also see
http://codereview.appspot.com/96125/show )

[Yhg1s]

Checked in the patch to fix the forks-through-os.fork() cases, which
should be most of them. Forks from other C code will need some more work,
created http://bugs.python.org/issue6923 to track that.

[BryanSchmersal]

I have a module that I was using on 2.5 that uses subprocess.Popen to monitor the output from some external programs in several different threads. Of course, subprocess.Popen uses os.fork. When I upgraded to 2.7 which includes this fix, this module ran into a deadlock since the fork is being executed from within an import. One could argue that my approach is poor style but one of the goals of this module is simplicity for the users....they simply need to import it to get the functionality of the module.

Was this a desired side-effect?