So attached (and at http://codereview.appspot.com/96125/show ) is a
preliminary fix, correcting the problem with os.fork(), os.forkpty() and
os.fork1(). This doesn't expose a general API for C code to use, for two
reasons: it's not easy, and I need this fix more than I need the API change
:-) (I actually need this fix myself for Python 2.4, but it applies fairly
cleanly.)
To fix the mutex-across-fork problem correctly we should really acquire
three locks (the import lock, the GIL and the TLS lock, in that order.) The
import lock is re-entrant, and the TLS lock is tightly confined to the
thread-local-storage lookup functions, but the GIL is neither re-entrant nor
inspectable. That means we can't use pthread_atfork (we can't tell whether
we have the GIL already or not, right before the fork), nor can we provide a
convenient API for extension modules to use, really. The best we can do is
provide three functions, pthread_atfork-style: a 'prepare' function, an
'after-fork-in-child' function, and an 'after-fork-in-parent' function. The
'prepare' function would start by releasing the GIL, then acquire the import
lock, the GIL and the TLS lock in that order. It would also record
*somewhere* the thread_ident of the current thread. The 'in-parent' function
would simply release the TLS lock and the import lock, and the 'in-child'
would release those locks, call the threading._at_fork() function, and fix
up the TLS data, using the recorded thread ident to do lookups. The
'in-child' function would replace the current PyOS_AfterFork() function
(which blindly reinitializes the GIL and the TLS lock, and calls
threading._at_fork().)
Alternatively we could do what we do now, which is to ignore the fact that
thread idents may change by fork() and thus that thread-local data may
disappear, in which case the 'in-child' function could do a little less
work. I'm suitably scared and disgusted of the TLS implementation, the
threading implementations we support (including the pthread one) and the way
we blindly type-pun a pthread_t to a long, that I'm not sure I want to try
and build something correct and reliable on top of it.
--
Thomas Wouters <[email protected]>
Hi! I'm a .signature virus! copy me into your .signature file to help me
spread!
Index: Python/import.c
===================================================================
--- Python/import.c (revision 74154)
+++ Python/import.c (working copy)
@@ -256,8 +256,8 @@
static long import_lock_thread = -1;
static int import_lock_level = 0;
-static void
-lock_import(void)
+void
+_PyImport_AcquireLock(void)
{
long me = PyThread_get_thread_ident();
if (me == -1)
@@ -281,8 +281,8 @@
import_lock_level = 1;
}
-static int
-unlock_import(void)
+int
+_PyImport_ReleaseLock(void)
{
long me = PyThread_get_thread_ident();
if (me == -1 || import_lock == NULL)
@@ -303,17 +303,8 @@
void
_PyImport_ReInitLock(void)
{
-#ifdef _AIX
- if (import_lock != NULL)
- import_lock = PyThread_allocate_lock();
-#endif
}
-#else
-
-#define lock_import()
-#define unlock_import() 0
-
#endif
static PyObject *
@@ -330,7 +321,7 @@
imp_acquire_lock(PyObject *self, PyObject *noargs)
{
#ifdef WITH_THREAD
- lock_import();
+ _PyImport_AcquireLock();
#endif
Py_INCREF(Py_None);
return Py_None;
@@ -340,7 +331,7 @@
imp_release_lock(PyObject *self, PyObject *noargs)
{
#ifdef WITH_THREAD
- if (unlock_import() < 0) {
+ if (_PyImport_ReleaseLock() < 0) {
PyErr_SetString(PyExc_RuntimeError,
"not holding the import lock");
return NULL;
@@ -2183,9 +2174,9 @@
PyObject *fromlist, int level)
{
PyObject *result;
- lock_import();
+ _PyImport_AcquireLock();
result = import_module_level(name, globals, locals, fromlist, level);
- if (unlock_import() < 0) {
+ if (_PyImport_ReleaseLock() < 0) {
Py_XDECREF(result);
PyErr_SetString(PyExc_RuntimeError,
"not holding the import lock");
Index: Include/import.h
===================================================================
--- Include/import.h (revision 74154)
+++ Include/import.h (working copy)
@@ -27,6 +27,14 @@
PyAPI_FUNC(void) PyImport_Cleanup(void);
PyAPI_FUNC(int) PyImport_ImportFrozenModule(char *);
+#ifdef WITH_THREAD
+PyAPI_FUNC(void) _PyImport_AcquireLock(void);
+PyAPI_FUNC(int) _PyImport_ReleaseLock(void);
+#else
+#define _PyImport_AcquireLock()
+#define _PyImport_ReleaseLock() 1
+#endif
+
PyAPI_FUNC(struct filedescr *) _PyImport_FindModule(
const char *, PyObject *, char *, size_t, FILE **, PyObject **);
PyAPI_FUNC(int) _PyImport_IsScript(struct filedescr *);
Index: Lib/test/test_fork1.py
===================================================================
--- Lib/test/test_fork1.py (revision 74154)
+++ Lib/test/test_fork1.py (working copy)
@@ -1,11 +1,18 @@
"""This test checks for correct fork() behavior.
"""
+import errno
+import imp
import os
+import signal
+import sys
import time
+import threading
+
from test.fork_wait import ForkWait
from test.test_support import TestSkipped, run_unittest, reap_children
+
try:
os.fork
except AttributeError:
@@ -24,6 +31,41 @@
self.assertEqual(spid, cpid)
self.assertEqual(status, 0, "cause = %d, exit = %d" % (status&0xff, status>>8))
+ def test_import_lock_fork(self):
+ import_started = threading.Event()
+ fake_module_name = "fake test module"
+ partial_module = "partial"
+ complete_module = "complete"
+ def importer():
+ imp.acquire_lock()
+ sys.modules[fake_module_name] = partial_module
+ import_started.set()
+ time.sleep(0.01) # Give the other thread time to try and acquire.
+ sys.modules[fake_module_name] = complete_module
+ imp.release_lock()
+ t = threading.Thread(target=importer)
+ t.start()
+ import_started.wait()
+ pid = os.fork()
+ try:
+ if not pid:
+ m = __import__(fake_module_name)
+ if m == complete_module:
+ os._exit(0)
+ else:
+ os._exit(1)
+ else:
+ t.join()
+ # Exitcode 1 means the child got a partial module (bad.) No
+ # exitcode (but a hang, which manifests as 'got pid 0')
+ # means the child deadlocked (also bad.)
+ self.wait_impl(pid)
+ finally:
+ try:
+ os.kill(pid, signal.SIGKILL)
+ except OSError:
+ pass
+
def test_main():
run_unittest(ForkTest)
reap_children()
Index: Modules/posixmodule.c
===================================================================
--- Modules/posixmodule.c (revision 74154)
+++ Modules/posixmodule.c (working copy)
@@ -3630,7 +3630,11 @@
static PyObject *
posix_fork1(PyObject *self, PyObject *noargs)
{
- pid_t pid = fork1();
+ pid_t pid;
+ _PyImport_AcquireLock();
+ pid = fork1();
+ /* XXX(twouters): check PyImport_ReleaseLock's return value? */
+ _PyImport_ReleaseLock();
if (pid == -1)
return posix_error();
if (pid == 0)
@@ -3649,7 +3653,11 @@
static PyObject *
posix_fork(PyObject *self, PyObject *noargs)
{
- pid_t pid = fork();
+ pid_t pid;
+ _PyImport_AcquireLock();
+ pid = fork();
+ /* XXX(twouters): check PyImport_ReleaseLock's return value? */
+ _PyImport_ReleaseLock();
if (pid == -1)
return posix_error();
if (pid == 0)
@@ -3759,7 +3767,10 @@
int master_fd = -1;
pid_t pid;
+ _PyImport_AcquireLock();
pid = forkpty(&master_fd, NULL, NULL, NULL);
+ /* XXX(twouters): check PyImport_ReleaseLock's return value? */
+ _PyImport_ReleaseLock();
if (pid == -1)
return posix_error();
if (pid == 0)
_______________________________________________
Python-Dev mailing list
[email protected]
http://mail.python.org/mailman/listinfo/python-dev
Unsubscribe:
http://mail.python.org/mailman/options/python-dev/archive%40mail-archive.com
