[pypy-commit] pypy cpyext-gil-ensure: in-progress (untested)
arigo
pypy.commits at gmail.com
Tue Mar 22 10:25:10 EDT 2016
Author: Armin Rigo <arigo at tunes.org>
Branch: cpyext-gil-ensure
Changeset: r83251:45bacc9b58c2
Date: 2016-03-22 15:24 +0100
http://bitbucket.org/pypy/pypy/changeset/45bacc9b58c2/
Log: in-progress (untested)
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
@@ -37,6 +37,8 @@
from rpython.tool.sourcetools import func_with_new_name
from rpython.rtyper.lltypesystem.lloperation import llop
from rpython.rlib import rawrefcount
+from rpython.rlib import rthread
+from rpython.rlib.debug import fatalerror_notb
DEBUG_WRAPPER = True
@@ -195,10 +197,10 @@
# Handling of the GIL
# -------------------
#
-# We add a global variable that contains a thread 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.
+# We add a global variable 'cpyext_glob_tid' that contains a thread
+# 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.
#
# **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
@@ -244,6 +246,9 @@
# into the global variable. The rest of the logic of
# PyGILState_Release() should be done before, in pystate.py.
+cpyext_glob_tid_ptr = lltype.malloc(rffi.CArray(lltype.Signed), 1,
+ flavor='raw', immortal=True, zero=True)
+
cpyext_namespace = NameManager('cpyext_')
@@ -668,7 +673,14 @@
fatal_value = callable.api_func.restype._defl()
gil_acquire = (gil == "acquire" or gil == "around")
gil_release = (gil == "release" or gil == "around")
- assert gil is None or gil_acquire or gil_release
+ pygilstate_ensure = (gil == "pygilstate_ensure")
+ pygilstate_release = (gil == "pygilstate_release")
+ assert (gil is None or gil_acquire or gil_release
+ or pygilstate_ensure or pygilstate_release)
+ deadlock_error = ("GIL deadlock detected when a CPython C extension "
+ "module calls back %r" % (callable.__name__,))
+ no_gil_error = ("GIL not held when a CPython C extension "
+ "module calls back %r" % (callable.__name__,))
@specialize.ll()
def wrapper(*args):
@@ -676,8 +688,27 @@
from pypy.module.cpyext.pyobject import as_pyobj
# we hope that malloc removal removes the newtuple() that is
# inserted exactly here by the varargs specializer
+
+ # see "Handling of the GIL" above (careful, we don't have the GIL here)
+ tid = rthread.get_or_make_ident()
if gil_acquire:
+ if cpyext_glob_tid_ptr[0] == tid:
+ fatalerror_notb(deadlock_error)
rgil.acquire()
+ assert cpyext_glob_tid_ptr[0] == 0
+ elif pygilstate_ensure:
+ from pypy.module.cpyext import pystate
+ if cpyext_glob_tid_ptr[0] == tid:
+ cpyext_glob_tid_ptr[0] = 0
+ args += (pystate.PyGILState_LOCKED,)
+ else:
+ rgil.acquire()
+ args += (pystate.PyGILState_UNLOCKED,)
+ else:
+ if cpyext_glob_tid_ptr[0] != tid:
+ fatalerror_notb(no_gil_error)
+ cpyext_glob_tid_ptr[0] = 0
+
rffi.stackcounter.stacks_counter += 1
llop.gc_stack_bottom(lltype.Void) # marker for trackgcroot.py
retval = fatal_value
@@ -754,8 +785,20 @@
pypy_debug_catch_fatal_exception()
assert False
rffi.stackcounter.stacks_counter -= 1
- if gil_release:
+
+ # see "Handling of the GIL" above
+ assert cpyext_glob_tid_ptr[0] == 0
+ if pygilstate_release:
+ from pypy.module.cpyext import pystate
+ arg = rffi.cast(lltype.Signed, args[-1])
+ unlock = (arg == pystate.PyGILState_UNLOCKED)
+ else:
+ unlock = gil_release
+ if unlock:
rgil.release()
+ else:
+ cpyext_glob_tid_ptr[0] = tid
+
return retval
callable._always_inline_ = 'try'
wrapper.__name__ = "wrapper for %r" % (callable, )
@@ -1401,10 +1444,17 @@
arg = as_pyobj(space, arg)
boxed_args += (arg,)
+ # see "Handling of the GIL" above
+ tid = rthread.get_ident()
+ assert cpyext_glob_tid_ptr[0] == 0
+ cpyext_glob_tid_ptr[0] = tid
+
try:
# Call the function
result = call_external_function(func, *boxed_args)
finally:
+ assert cpyext_glob_tid_ptr[0] == tid
+ cpyext_glob_tid_ptr[0] = 0
keepalive_until_here(*keepalives)
if is_PyObject(RESULT_TYPE):
diff --git a/pypy/module/cpyext/pystate.py b/pypy/module/cpyext/pystate.py
--- a/pypy/module/cpyext/pystate.py
+++ b/pypy/module/cpyext/pystate.py
@@ -204,18 +204,43 @@
compile time."""
PyGILState_STATE = rffi.INT
+PyGILState_LOCKED = 0
+PyGILState_UNLOCKED = 1
- at cpython_api([], PyGILState_STATE, error=CANNOT_FAIL, gil="acquire")
-def PyGILState_Ensure(space):
- # XXX XXX XXX THIS IS A VERY MINIMAL IMPLEMENTATION THAT WILL HAPPILY
- # DEADLOCK IF CALLED TWICE ON THE SAME THREAD, OR CRASH IF CALLED IN A
- # NEW THREAD. We should very carefully follow what CPython does instead.
- return rffi.cast(PyGILState_STATE, 0)
+ExecutionContext.cpyext_gilstate_counter_noleave = 0
- at cpython_api([PyGILState_STATE], lltype.Void, gil="release")
-def PyGILState_Release(space, state):
- # XXX XXX XXX We should very carefully follow what CPython does instead.
- pass
+ at cpython_api([], PyGILState_STATE, error=CANNOT_FAIL, gil="pygilstate_ensure")
+def PyGILState_Ensure(space, previous_state):
+ # The argument 'previous_state' is not part of the API; it is inserted
+ # by make_wrapper() and contains PyGILState_LOCKED/UNLOCKED based on
+ # the previous GIL state.
+ must_leave = space.threadlocals.try_enter_thread(space)
+ ec = space.getexecutioncontext()
+ if not must_leave:
+ # This is a counter of how many times we called try_enter_thread()
+ # and it returned False. In PyGILState_Release(), if this counter
+ # is greater than zero, we decrement it; only if the counter is
+ # already zero do we call leave_thread().
+ ec.cpyext_gilstate_counter_noleave += 1
+ else:
+ # This case is for when we just built a fresh threadlocals.
+ # We should only see it when we are in a new thread with no
+ # PyPy code below.
+ assert previous_state == PyGILState_UNLOCKED
+ assert ec.cpyext_gilstate_counter_noleave == 0
+ #
+ return rffi.cast(PyGILState_STATE, previous_state)
+
+ at cpython_api([PyGILState_STATE], lltype.Void, gil="pygilstate_release")
+def PyGILState_Release(space, oldstate):
+ oldstate = rffi.cast(lltype.Signed, oldstate)
+ ec = space.getexecutioncontext()
+ if ec.cpyext_gilstate_counter_noleave > 0:
+ ec.cpyext_gilstate_counter_noleave -= 1
+ else:
+ assert ec.cpyext_gilstate_counter_noleave == 0
+ assert oldstate == PyGILState_UNLOCKED
+ space.threadlocals.leave_thread(space)
@cpython_api([], PyInterpreterState, error=CANNOT_FAIL)
def PyInterpreterState_Head(space):
diff --git a/rpython/rlib/rthread.py b/rpython/rlib/rthread.py
--- a/rpython/rlib/rthread.py
+++ b/rpython/rlib/rthread.py
@@ -100,8 +100,11 @@
return thread.get_ident()
def get_or_make_ident():
- assert we_are_translated()
- return tlfield_thread_ident.get_or_make_raw()
+ if we_are_translated():
+ return tlfield_thread_ident.get_or_make_raw()
+ else:
+ import thread
+ retrun thread.get_ident()
@specialize.arg(0)
def start_new_thread(x, y):
More information about the pypy-commit
mailing list