[Python-Dev] pthreads, fork, import, and execvp

[Nick Coghlan]

Martin v. Löwis wrote:
> So if the the import lock was held during the fork call, the new thread
> will hold the import lock of the new process, and subsequent imports
> will block.

> However, then the problem is not with the execve implementation, but
> with the fact that the import lock was held when the process forked.
> 
> Rotem should simply avoid to fork() in the toplevel code of a module.

That's what I originally thought, but Rotem was able to clarify for me that 
the problem occurs even if the import lock is held by a *different* thread at 
the time fork() is called. So a deadlock can be triggered by perfectly 
legitimate code that has the misfortune to fork the process while another 
thread is doing a module import.

> Unfortunately, that fix would apply to AIX only, though:
> 
> void
> _PyImport_ReInitLock(void)
> {
> #ifdef _AIX
>         if (import_lock != NULL)
>                 import_lock = PyThread_allocate_lock();
> #endif
> }
> 
> If we want to support fork while an import is going on, we should
> release the import lock if it is held. Alternatively, the code
> could throw away the old import lock on all systems; that seems
> like a waste of resources to me, though. One should also reset
> import_lock_thread and import_lock_level.

Broadening the ifdef to cover all posix systems rather than just AIX might be 
the right thing to do. However, the first thing would be to reproduce the 
deadlock reliably.

A repeatable test case would look something like this:

class ForkingThread(Thread):
     def run(self):
         # create a subprocess using subprocess.Popen
         # and ensure it tries to do an import operation
         # (i.e. something similar to what Rotem & Yair's code is doing)

class ImportingThread(Thread):
     def __init__(self):
         Thread.__init__(self)
         self.importing = Event()
         self.cleanup = Event()
     def run(self):
         imp.acquire_lock()
         try:
             self.importing.set()
             self.cleanup.wait()
         finally:
             imp.release_lock()

def test_forked_import_deadlock():
     import_thread = ImportingThread()
     fork_thread = ForkingThread()
     import_thread.start()
     try:
         import_thread.importing.wait()
         fork_thread.start()   # Fork while the import thread holds the lock
         fork_thread.join(10)  # Give the subprocess time to finish
         assert not fork_thread.isAlive() # A timeout means it deadlocked
     finally:
         import_thread.cleanup.set() # Release the import lock

Cheers,
Nick.

-- 
Nick Coghlan   |   ncoghlan at gmail.com   |   Brisbane, Australia
---------------------------------------------------------------
             http://www.boredomandlaziness.org