Author: Armin Rigo <[email protected]>
Branch: cpyext-avoid-roundtrip
Changeset: r93441:56643108f56a
Date: 2017-12-16 10:33 +0100
http://bitbucket.org/pypy/pypy/changeset/56643108f56a/
Log: Test and fix for the case where state.C.Xxx calls directly some C
function which tries to call back into RPython
diff --git a/pypy/module/cpyext/api.py b/pypy/module/cpyext/api.py
--- a/pypy/module/cpyext/api.py
+++ b/pypy/module/cpyext/api.py
@@ -204,6 +204,9 @@
# id. Invariant: this variable always contain 0 when the PyPy GIL is
# released. It should also contain 0 when regular RPython code
# executes. In non-cpyext-related code, it will thus always be 0.
+# When cpyext-related C code runs, it contains the thread id (usually)
+# or the value -1 (only for state.C.PyXxx() functions which are short-
+# running and should not themselves release the GIL).
#
# **make_generic_cpy_call():** RPython to C, with the GIL held. Before
# the call, must assert that the global variable is 0 and set the
@@ -972,8 +975,14 @@
# see "Handling of the GIL" above (careful, we don't have the GIL here)
tid = rthread.get_or_make_ident()
- _gil_auto = (gil_auto_workaround and cpyext_glob_tid_ptr[0] != tid)
- if gil_acquire or _gil_auto:
+ _gil_auto = False
+ if gil_auto_workaround and cpyext_glob_tid_ptr[0] != tid:
+ # replace '-1' with the real tid, now that we have the tid
+ if cpyext_glob_tid_ptr[0] == -1:
+ cpyext_glob_tid_ptr[0] = tid
+ else:
+ _gil_auto = True
+ if _gil_auto or gil_acquire:
if cpyext_glob_tid_ptr[0] == tid:
deadlock_error(pname)
rgil.acquire()
diff --git a/pypy/module/cpyext/intobject.py b/pypy/module/cpyext/intobject.py
--- a/pypy/module/cpyext/intobject.py
+++ b/pypy/module/cpyext/intobject.py
@@ -35,7 +35,7 @@
# value. However, it's just easier to call PyInt_FromLong with a dummy
# value; make sure it's big enough to avoid the smallint optimization
# (if it will ever be enabled)
- return state.C.PyInt_FromLong(0x0DEADBEE)
+ return state.ccall("PyInt_FromLong", 0x0DEADBEE)
else:
return BaseCpyTypedescr.allocate(typedescr, space, w_type, itemcount)
diff --git a/pypy/module/cpyext/pyerrors.py b/pypy/module/cpyext/pyerrors.py
--- a/pypy/module/cpyext/pyerrors.py
+++ b/pypy/module/cpyext/pyerrors.py
@@ -123,7 +123,9 @@
error indicator."""
raise oefmt(space.w_TypeError, "bad argument type for built-in operation")
-@cpython_api([], lltype.Void)
+# NB. this returns 'void' in CPython, but we can't easily, otherwise the
+# function is supposed not to fail
+@cpython_api([], rffi.INT_real, error=-1)
def PyErr_BadInternalCall(space):
raise oefmt(space.w_SystemError, "Bad internal call!")
diff --git a/pypy/module/cpyext/pyobject.py b/pypy/module/cpyext/pyobject.py
--- a/pypy/module/cpyext/pyobject.py
+++ b/pypy/module/cpyext/pyobject.py
@@ -307,7 +307,7 @@
if w_obj is not None and space.type(w_obj) is space.w_int:
state = space.fromcache(State)
intval = space.int_w(w_obj)
- return state.C.PyInt_FromLong(intval)
+ return state.ccall("PyInt_FromLong", intval)
return get_pyobj_and_incref(space, w_obj, w_userdata, immortal=False)
@specialize.ll()
diff --git a/pypy/module/cpyext/state.py b/pypy/module/cpyext/state.py
--- a/pypy/module/cpyext/state.py
+++ b/pypy/module/cpyext/state.py
@@ -1,4 +1,4 @@
-from rpython.rlib.objectmodel import we_are_translated
+from rpython.rlib.objectmodel import we_are_translated, specialize
from rpython.rtyper.lltypesystem import rffi, lltype
from pypy.interpreter.error import OperationError, oefmt
from pypy.interpreter import executioncontext
@@ -45,6 +45,7 @@
self.operror = None
return operror
+ @specialize.arg(1)
def check_and_raise_exception(self, always=False):
operror = self.operror
if operror:
@@ -168,6 +169,18 @@
self.extensions[path] = w_copy
return w_mod
+ @specialize.arg(1)
+ def ccall(self, name, *args):
+ from pypy.module.cpyext.api import cpyext_glob_tid_ptr
+ # This is similar to doing a direct call to state.C.PyXxx(), but
+ # must be used for any function that might potentially call back
+ # RPython code---most of them can, e.g. PyErr_NoMemory().
+ assert cpyext_glob_tid_ptr[0] == 0
+ cpyext_glob_tid_ptr[0] = -1
+ result = getattr(self.C, name)(*args)
+ cpyext_glob_tid_ptr[0] = 0
+ return result
+
class CNamespace:
def _freeze_(self):
diff --git a/pypy/module/cpyext/test/test_tupleobject.py
b/pypy/module/cpyext/test/test_tupleobject.py
--- a/pypy/module/cpyext/test/test_tupleobject.py
+++ b/pypy/module/cpyext/test/test_tupleobject.py
@@ -144,6 +144,46 @@
space.newtuple([space.wrap(i) for i in range(3, 7)]))
+class AppTestBadInternalCall(AppTestCpythonExtensionBase):
+ def setup_method(self, meth):
+ from pypy.module.cpyext import tupleobject
+ if self.runappdirect:
+ py.test.skip("not for runappdirect")
+ AppTestCpythonExtensionBase.setup_method(self, meth)
+ tupleobject._BAD_ITEMCOUNT = 42
+
+ def teardown_method(self, meth):
+ from pypy.module.cpyext import tupleobject
+ AppTestCpythonExtensionBase.teardown_method(self, meth)
+ tupleobject._BAD_ITEMCOUNT = None
+
+ def test_badinternalcall_from_rpy(self):
+ # This used to hit "a thread is trying to wait for the GIL" in
+ # thread_gil.c
+ module = self.import_extension('foo', [
+ ("badinternalcall2", "METH_O",
+ """
+ Py_INCREF(Py_None);
+ return Py_None;
+ """),
+ ])
+ tup = (None,) * 42
+ raises(SystemError, module.badinternalcall2, tup)
+
+ def test_badinternalcall_from_rpy_with_threads(self):
+ # This used to cause a deadlock in thread_gil.c
+ module = self.import_extension('foo', [
+ ("badinternalcall2", "METH_O",
+ """
+ Py_INCREF(Py_None);
+ return Py_None;
+ """),
+ ])
+ import thread; thread.start_new_thread(lambda: None, ())
+ tup = (None,) * 42
+ raises(SystemError, module.badinternalcall2, tup)
+
+
class AppTestTuple(AppTestCpythonExtensionBase):
def test_refcounts(self):
module = self.import_extension('foo', [
diff --git a/pypy/module/cpyext/tupleobject.py
b/pypy/module/cpyext/tupleobject.py
--- a/pypy/module/cpyext/tupleobject.py
+++ b/pypy/module/cpyext/tupleobject.py
@@ -1,5 +1,6 @@
from pypy.interpreter.error import oefmt
from rpython.rtyper.lltypesystem import rffi, lltype
+from rpython.rlib.objectmodel import we_are_translated
from rpython.rlib.debug import fatalerror_notb
from pypy.module.cpyext.api import (
cpython_api, Py_ssize_t, build_type_checkers_flags,
@@ -56,10 +57,17 @@
return (w_type is space.w_tuple or
space.issubtype_w(w_type, space.w_tuple))
+_BAD_ITEMCOUNT = None # patched in test_badinternalcall_from_rpy
+
def tuple_alloc(typedescr, space, w_type, itemcount):
state = space.fromcache(State)
if w_type is space.w_tuple:
- return state.C.PyTuple_New(itemcount)
+ if not we_are_translated() and itemcount == _BAD_ITEMCOUNT:
+ itemcount = -42
+ ptup = state.ccall("PyTuple_New", itemcount)
+ if not ptup:
+ state.check_and_raise_exception(always=True)
+ return ptup
else:
return BaseCpyTypedescr.allocate(typedescr, space, w_type, itemcount)
@@ -116,7 +124,9 @@
def tuple_from_args_w(space, args_w):
state = space.fromcache(State)
n = len(args_w)
- py_tuple = state.C.PyTuple_New(n) # XXX: check for errors?
+ py_tuple = state.ccall("PyTuple_New", n)
+ if not py_tuple:
+ state.check_and_raise_exception(always=True)
py_tuple = rffi.cast(PyTupleObject, py_tuple)
for i, w_obj in enumerate(args_w):
py_tuple.c_ob_item[i] = make_ref(space, w_obj)
@@ -181,7 +191,10 @@
PyErr_BadInternalCall(space)
oldref = rffi.cast(PyTupleObject, ref)
oldsize = oldref.c_ob_size
- p_ref[0] = state.C.PyTuple_New(newsize)
+ ptup = state.ccall("PyTuple_New", newsize)
+ if not ptup:
+ state.check_and_raise_exception(always=True)
+ p_ref[0] = ptup
newref = rffi.cast(PyTupleObject, p_ref[0])
try:
if oldsize < newsize:
_______________________________________________
pypy-commit mailing list
[email protected]
https://mail.python.org/mailman/listinfo/pypy-commit
