Forking in a thread raises RuntimeError

[csernazs]

BPO	7242
Nosy	@loewis, @Yhg1s, @gpshead, @csernazs, @pitrou, @dhellmann
Files	thread_test.py: A test demonstrating the issue. patch_1.diff: A possible fix for this issue created for python 2.6.4. patch_2.diff: Patch re-initializing import_lock on all platforms after fork(). thread_unittest.py: Unittest for the patch 7242_unittest.diff: Unit test that tests the issue issue7242-gps01.diff: update of patch_2 + thread_unittest

^{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/gpshead'
closed_at = <Date 2010-03-01.04:41:35.268>
created_at = <Date 2009-10-30.10:58:01.329>
labels = ['extension-modules', 'type-bug']
title = 'Forking in a thread raises RuntimeError'
updated_at = <Date 2010-12-18.16:55:12.147>
user = 'https://github.com/csernazs'

bugs.python.org fields:

activity = <Date 2010-12-18.16:55:12.147>
actor = 'Oren_Held'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2010-03-01.04:41:35.268>
closer = 'gregory.p.smith'
components = ['Extension Modules']
creation = <Date 2009-10-30.10:58:01.329>
creator = 'csernazs'
dependencies = []
files = ['15232', '15247', '16011', '16378', '16381', '16395']
hgrepos = []
issue_num = 7242
keywords = ['patch']
message_count = 26.0
messages = ['94701', '94702', '94704', '94765', '94820', '94822', '94832', '95075', '98148', '98211', '98287', '98299', '98335', '99202', '100090', '100100', '100143', '100144', '100171', '100200', '100204', '100211', '100231', '100233', '100245', '124294']
nosy_count = 9.0
nosy_names = ['loewis', 'twouters', 'gregory.p.smith', 'csernazs', 'pitrou', 'doughellmann', 'dagobert', 'Oren_Held', 'jednaszewski']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue7242'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2']

[csernazs]

Python 2.6.4 (r264:75706, Oct 29 2009, 12:00:12) [C] on sunos5

On my sunos5 system if I call os.fork() in a thread created by
thread.start_new_thread(), it raises RuntimeError with the message 'not
holding the import lock'. It's a vanilla python compiled on sunos5 with
suncc. In 2.6.3 it's ok, I think this issue is caused by patch located
at http://codereview.appspot.com/96125/show.

The problem can be re-produced by running the script attached.

I've looked into the source and it seems to me the following:

Based on the the change above, it calls fork1() system call between a
lock-release calls:

3635 » _PyImport_AcquireLock();
3636 » pid = fork1();
3637 » result = _PyImport_ReleaseLock();

_PyImport_ReleaseLock is called in both the child and parent. Digging
more into the code, _PyImport_ReleaseLock starts with the following:

    long me = PyThread_get_thread_ident();
    if (me == -1 || import_lock == NULL)
            return 0; /* Too bad */
    if (import_lock_thread != me)
            return -1;

In the above code, if I interpret correctly, it compares the result of
the current thread id returned by PyThread_get_thread_ident call with
the thread id of the thread holding the lock - if it's different then
the current thread cannot release the lock because it's not owned by it.

Based on my results on solaris the 'me' variable will be different in
the parent and in the child (in parent it remains the same) - resulting
that the child thinks that it's not holding the release lock.

I'm using pthreads on both linux and solaris. On linux the code above is
working fine, but on solaris it behaves differently.

[pitrou]

Well, there has been no such change between 2.6.3 and 2.6.4.

[csernazs]

Sorry, the working version is not 2.6.3 (I mistyped the version), it's
2.6.1 (I've no info about 2.6.2).

[gpshead]

This only appears to happen on Solaris. What version of Solaris are you
using? (i doubt that matters, i expect it happens on all versions)

I haven't look closely enough at the code yet, but reinitializing the
import lock in the child process should make sense here.

[csernazs]

I've tested it only on solaris 8, 32-bit.

[csernazs]

solaris 10 x86, 32-bit, sun-studio 11 is ok (in this case the parent's
thread has thread_id=2 and the child inherits this id)
solaris 8 sparc4, 32-bit, sun-studio 11 is not working

So it seems it's independent from sun-cc but depends from the
architecture and/or the OS.

[csernazs]

I've attached a patch which seems to fix this issue. It sets
import_lock_thread to the current thread id after forking in the child
process, but still I'm not quite sure that it's the correct way of
solving this issue.

[csernazs]

Additional info:
I've tested it on solaris 10 / sparc 32-bit, and my test script runs
fine on that.
Based on my test it seems that this bug does not affect solaris 10.

[csernazs]

Could you please provide any advise on this bug?
I've submitted my patch, would be curious if there any chance to get it merged.

[loewis]

One relevant difference may be the change to the fork semantics in Solaris 10: fork() now means the same as fork1(). Before, fork() meant the same as forkall(), unless the applications was linked with -lpthread, in which case fork() also meant fork1(). See fork(2).

So I'd be curious if a) the test case passes on Solaris 8 if fork1 is being used, and b) whether it passes if you make sure -lpthread is being used. If so, I'd rather fix this issue by changing the linkage for Solaris 8 (or declaring that system as unsupported altogether), instead of fiddling yet again with the fork handling. In Python, posix.fork definitely needs to mean "POSIX fork".

If it's something else (i.e. the thread ID changes in Solaris 8 whether fork1 or forkall is used), then the patch is fine in principle. However, I would always reset the thread ID. In your patch, it is not clear why you apply this resetting to Solaris and AIX, but not other systems.

[csernazs]

I compile it with -lpthread.
os.fork1() was not available by default, I enabled it by removing two lines from posixmodule.c (it seems it's only enabled when #if defined(USLC) && defined(SCO_VERSION) is true).

With os.fork1() I have the same results, RuntimeError.

I was not able to compile it without pthread because I haven't found any configure options for that. If it's possible I'm happy to try it.

In my patch I wanted to reduce the effect on systems where forking in thread is working (eg. linux), that's the reason why I added "(defined (__SVR4) && defined (__sun)". But it just checks for solaris, not the OS version (on solaris 10/intel my demo is working).

(btw forking in thread actually happens in a unittest related to BaseHTTPServer, which obviously fails on my platform)

[loewis]

In my patch I wanted to reduce the effect on systems where forking in
thread is working (eg. linux), that's the reason why I added
"(defined (__SVR4) && defined (__sun)".

I think that's inappropriate, please change that.