We have a Python HTTP server that, in the parent
process, uses os.fork to spawn new children, but at the
same time the parent could be spawning new threads (in
threads other than the main thread -- only the main
thread does forking).

Anwyay, it very rarely causes deadlock in a spawned
child when that child tries to start a new thread.

I believe I've tracked this down to the
_active_limbo_lock in the threading module.
Specifically, if this lock is held by parent (because
another thread is spawning a thread), just as os.fork
happens, the child inherits the held lock and will then
block trying to do any threading.* operations.

Just like the global interp. lock is overwritten in the
child after fork, I think something similar should
happen to threading._active_limbo_lock? (And more
generally the user needs to be aware of locks passing
through fork; but I think at least threading.py should
"do the right thing").

This thread looks quite relevant:

groups.google.com/groups?hl=en&lr=&ie=UTF-8&oe=UTF-8&threadm=38E6F2BA.E66CAC90%40ensim.com&rnum=5&prev=/groups%3Fq%3Dpython%2Bfork%2Bthreading%2Bmodule%2B%2Block%26hl%3Den%26lr%3D%26ie%3DUTF-8%26oe%3DUTF-8%26sa%3DN%26scoring%3Dd

Logged In: YES
user_id=539787

See some more typical info about mixing forks and threads:
http://mail.python.org/pipermail/python-list/2001-September/066048.html

This seems not to be Python-related, but platform-related.

Logged In: YES
user_id=323786

True -- there are many platform specific issues on the
interaction of forking and threading.

However the _active_limbo_lock is entirely a Python issue (I
think it can deadlock on any platform, unless the platform
releases locks in child process after fork).

After forking, python already resets the GIL, and it should
also reset any other locks that have global impact like
_active_limbo_lock.

It's not only the _active_limbo_lock. All global structures of the
threading module should be reinitialized (including the _MainThread
instance); for that purpose, reload() can be used. I attach an example
which exercises this problem. Normally the script should hang in the
last step (using os.fork() and launching threads while some other
threads call threading.enumerate() in a loop).

The safe_fork() function in the example is a replacement for os.fork()
which tries to avoid any deadlock in the threading module. If it's
deemed useful and bug-free, it could perhaps be added somewhere in the
stdlib (in the threading module itself perhaps).

(or perhaps we should provide an API to hook into PyOS_AfterFork)

Attached patch releases the _active_limbo_lock after a fork(). It is not
a complete solution, since existing Thread objects don't correspond to
anything, but it corrects a problem in test_multiprocessing.

A slightly better patch, with tests.

Amaury - your latest patch doesn't seem to apply cleanly to trunk, it's
choking on the Python/ceval.c changes

Here is a third patch with latest trunk.
Did you apply it to a clean checkout?

I had to flip on ignore whitespace to patch:
patch -p0 -l <~/Desktop/fork-and-thread3.patch

Worked. Unfortunately, test_threading is locking up during a make test.
Here's the verbose regrtest.py output:

woot:python-trunk jesse$ ./python.exe Lib/test/regrtest.py -v
test_threading
...snip
<thread 9> done
<thread 9> is finished. 0 tasks are running
all tasks done
ok
test_join_in_forked_from_thread
(test.test_threading.ThreadJoinOnShutdown) ...

At this point it hangs (OS/X 10.5 latest)

Amaury, it looks like it's hanging in subprocess:

/Users/jesse/open_source/subversion/python-
trunk/Lib/test/test_threading.py(338)_run_and_join()
-> p = subprocess.Popen([sys.executable, "-c", script],
stdout=subprocess.PIPE)
(Pdb) step
...snip...

(Pdb) step
--Call--

/Users/jesse/open_source/subversion/python-
trunk/Lib/threading.py(788)current_thread()
-> def current_thread():
(Pdb) step
/Users/jesse/open_source/subversion/python-
trunk/Lib/threading.py(789)current_thread()
-> try:
(Pdb) step
/Users/jesse/open_source/subversion/python-
trunk/Lib/threading.py(790)current_thread()
-> return _active[_get_ident()]
(Pdb) step
/Users/jesse/open_source/subversion/python-
trunk/Lib/subprocess.py(1097)_execute_child()
-> data = os.read(errpipe_read, 1048576) # Exceptions limited to 1 MB
(Pdb) step
... lockup

Here's the good news, with the patch applied to trunk, I'm not seeing
hangs in the multiprocessing test suite. I'm running it on a tight loop
excluding the threading tests to confirm.

Well I was a bit presumptuous that my thread+fork tests would pass on
all platforms out of the box.
We should disable the tests, or have someone with better Unix expertise
examine and correct them.
At least I feel that the actual correction in threading.py goes in the
right direction.

In general I suggest replacing the lock with a new lock, rather than
trying to release the existing one. Releasing *might* work in this
case, only because it's really a semaphore underneath, but it's still
easier to think about by just replacing.

I also suggest deleting _active and recreating it with only the current
thread.

I don't understand how test_join_on_shutdown could succeed. The main
thread shouldn't be marked as done.. well, ever. The test should hang.

I suspect test_join_in_forked_process should call os.waitpid(childpid)
so it doesn't exit early, which would cause the original Popen.wait()
call to exit before the output is produced. The same problem of
test_join_on_shutdown also applies.

Ditto for test_join_in_forked_from_thread.

Looking over some of the other platforms for thread_*.h, I'm sure
replacing the lock is the right thing.

A new patch:

• I replaced "_active_limbo_lock.release()" by
  "_active_limbo_lock=_allocate_lock()"

• I replaced the successive deletions in _active by a re-creation with
  only the current thread. There is no difference in the result, but I
  agree that the intent is more clear.

• yes, the main thread is marked as done when the interpreter exits
  (hence the convoluted tests with subprocesses): in Modules/main.c,
  WaitForThreadShutdown() calls threading._shutdown().

Also, I hope the tests make more sense now.

FWIW: the threading tests still hang for me with the latest patch. I'm
confirming that the patch itself fixes the hanging MP tests though

I can confirm that this seems to clear up the test_multiprocessing hangs
on my mac, I ran make test in a loop almost all day yesterday without
hangs. I would really suggest we get this in, minus the new test_threading
tests for now.

I agree. My attempt to describe the behaviour of fork+threads in a
platform-independent test is another issue.

Just one more thing: looking at Module/posixmodule.c, os.fork1() calls
PyOS_AfterFork() in both parent and child processes. Is there a "if (pid
== 0)" test missing, just like os.fork()?

I am leaving for the whole next week, so anyone, feel free to commit
(part of) my patch if it helps.

The existing fork-and-thread4.patch doesn't actually reset
_active_limbo_lock. Its missing a "global _active_limbo_lock" statement
in the threading._after_fork() function.

and a few more bugs. a new patch is attached. With this applied,
pitrou's fork_threading.py bug demonstration script finally does the
right thing.

test_threading, including the new deadlock tests, and
test_multiprocessing both pass.

Tested on x86 MacOS X 10.4 & x86 Ubuntu 8.04.

Would someone please try this on other platforms?

(The new threading test's use of subprocess with [sys.executable, '-c',
"""long script"""] makes me slightly nervous about portability outside
of Linux and BSD.)

I still don't like the _after_fork() implementation. Its O(n) where n
== number of threads the parent process had.

Very wasteful when the fork() was done in the most common case of being
followed by an exec and calling os._exit(). It won't scale nicely with
system load (forks will start taking longer and longer the more threads
exist).

Could os.fork() be extended to have an optional will_exec_or_die
parameter that determines if _after_fork() is even called at all?
Things like subprocess should pass in True. The default should be False
for compatiblity.

One of tests is hanging in 3.0.

specifically, test_3_join_in_forked_from_thread hangs in py3k and is
currently disabled.

If you remove the print() call from joining_func(), does it stop
hanging? It may be due to differences between the io library in py3k and
the builtin file objects in 2.x.
(looking at the date of the commit disabling the test, it is not related
to the thread-safety changes to BufferedWriter, though)

This is also causing hangs in 2.5.

it passes on release25-maint for me on linux. i don't see it hanging in
any of the 2.5 buildbots. looks like your r66003 change was a decent
fix for windows judging by the buildbot.

(fwiw, that test you patched in 66003 should probably use readlines()
and assertListEqual to be cross platform rather than the code thats
there now but i'm not motivated to change that unless some other
platform fails on it)

Ok, with this patch the test passes under py3k.

Apart from the trivial typo (_Thread__stopped -> _stopped), I had to
change print("...") to os.write(1, b"...\n") in the tests as otherwise
subprocess wouldn't receive any output from the third test (buffering
problem? I don't know really).

I also added the ident fix I had already suggested in _after_fork(). If
you put a print statement at this point you'll see the old and the new
value are not the same.

I feel like that patch sort of avoids the problem by changing the test.
The test is hanging for some reason, so we should try to fix that, not
the test. :) I wonder if it has something to do with the various
deadlocks we are discovering in the io library.

Benjamin, if you don't change the test, the deadlock problem is still
solved, it's just that the third test fails because the subprocess
stdout is empty instead of containing the desired value. It is *not*
because the subprocess doesn't print anything (if you launch an
equivalent program on the command line, everything is printed), rather
it seems that subprocess doesn't get what is printed from the child
process of the subprocess.

On Thu, Sep 4, 2008 at 6:08 PM, Antoine Pitrou <report@bugs.python.org> wrote:

Antoine Pitrou <pitrou@free.fr> added the comment:

Benjamin, if you don't change the test, the deadlock problem is still
solved, it's just that the third test fails because the subprocess
stdout is empty instead of containing the desired value. It is *not*
because the subprocess doesn't print anything (if you launch an
equivalent program on the command line, everything is printed), rather
it seems that subprocess doesn't get what is printed from the child
process of the subprocess.

Ah! My apologies for the giant misunderstanding.

Instead of os.write(), it is actually sufficient to sys.stdout.flush()
at the end of the subprocess. Patch attached.

Just checked that the latest patch works on Windows as well.

forkthread2.patch looks good to me. to be consistent shouldn't we also
apply that fix to 2.6?

Le samedi 06 septembre 2008 à 22:27 +0000, Gregory P. Smith a écrit :

Gregory P. Smith <greg@krypto.org> added the comment:

forkthread2.patch looks good to me. to be consistent shouldn't we also
apply that fix to 2.6?

Only the ident-resetting part needs to be backported, the rest is
py3k-specific (especially _stopped vs. _Thread__stopped :-)).

Committed in r66274, r66275. Thanks!

I backported the last bit from r66275 to release25-maint in r66279.