Merge remote branch 'markflorisson/withgil'

Conflicts:
	tests/errors/nogil.pyx

Cython/Compiler/Code.pxd

@@ -35,6 +35,7 @@ cdef class FunctionState:
     cdef public size_t temp_counter
 
     cdef public object closure_temps
+    cdef public bint should_declare_error_indicator
 
     @cython.locals(n=size_t)
     cpdef new_label(self, name=*)

Cython/Compiler/Code.py

@@ -134,6 +134,12 @@ class FunctionState(object):
         self.temp_counter = 0
         self.closure_temps = None
 
+        # This is used for the error indicator, which needs to be local to the
+        # function. It used to be global, which relies on the GIL being held.
+        # However, exceptions may need to be propagated through 'nogil'
+        # sections, in which case we introduce a race condition.
+        self.should_declare_error_indicator = False
+
     # labels
 
     def new_label(self, name=None):
@@ -1362,6 +1368,52 @@ class CCodeWriter(object):
                     doc_code,
                     term))
 
+    # GIL methods
+
+    def put_ensure_gil(self, declare_gilstate=True):
+        """
+        Acquire the GIL. The generated code is safe even when no PyThreadState
+        has been allocated for this thread (for threads not initialized by
+        using the Python API). Additionally, the code generated by this method
+        may be called recursively.
+        """
+        from Cython.Compiler import Nodes
+
+        self.globalstate.use_utility_code(Nodes.force_init_threads_utility_code)
+
+        self.putln("#ifdef WITH_THREAD")
+        if declare_gilstate:
+            self.put("PyGILState_STATE ")
+        self.putln("_save = PyGILState_Ensure();")
+        self.putln("#endif")
+
+    def put_release_ensured_gil(self):
+        """
+        Releases the GIL, corresponds to `put_ensure_gil`.
+        """
+        self.putln("#ifdef WITH_THREAD")
+        self.putln("PyGILState_Release(_save);")
+        self.putln("#endif")
+
+    def put_acquire_gil(self):
+        """
+        Acquire the GIL. The thread's thread state must have been initialized
+        by a previous `put_release_gil`
+        """
+        self.putln("Py_BLOCK_THREADS")
+
+    def put_release_gil(self):
+        "Release the GIL, corresponds to `put_acquire_gil`."
+        self.putln("#ifdef WITH_THREAD")
+        self.putln("PyThreadState *_save = NULL;")
+        self.putln("#endif")
+        self.putln("Py_UNBLOCK_THREADS")
+
+    def declare_gilstate(self):
+        self.putln("#ifdef WITH_THREAD")
+        self.putln("PyGILState_STATE _save;")
+        self.putln("#endif")
+
     # error handling
 
     def put_error_if_neg(self, pos, value):
@@ -1369,10 +1421,12 @@ class CCodeWriter(object):
         return self.putln("if (%s < 0) %s" % (value, self.error_goto(pos)))
 
     def set_error_info(self, pos):
+        self.funcstate.should_declare_error_indicator = True
         if self.c_line_in_traceback:
             cinfo = " %s = %s;" % (Naming.clineno_cname, Naming.line_c_macro)
         else:
             cinfo = ""
+
         return "%s = %s[%s]; %s = %s;%s" % (
             Naming.filename_cname,
             Naming.filetable_cname,
@@ -1412,6 +1466,20 @@ class CCodeWriter(object):
     def put_finish_refcount_context(self):
         self.putln("__Pyx_RefNannyFinishContext();")
 
+    def put_add_traceback(self, qualified_name):
+        """
+        Build a Python traceback for propagating exceptions.
+
+        qualified_name should be the qualified name of the function
+        """
+        format_tuple = (
+            qualified_name,
+            Naming.clineno_cname,
+            Naming.lineno_cname,
+            Naming.filename_cname,
+        )
+        self.putln('__Pyx_AddTraceback("%s", %s, %s, %s);' % format_tuple)
+
     def put_trace_declarations(self):
         self.putln('__Pyx_TraceDeclarations');
 

Cython/Compiler/ExprNodes.py

@@ -6308,7 +6308,12 @@ class DivNode(NumBinopNode):
                                 self.operand1.result(),
                                 self.operand2.result()))
                 code.putln(code.set_error_info(self.pos));
-                code.put("if (__Pyx_cdivision_warning()) ")
+                code.put("if (__Pyx_cdivision_warning(%(FILENAME)s, "
+                                                     "%(LINENO)s)) " % {
+                    'FILENAME': Naming.filename_cname,
+                    'LINENO':  Naming.lineno_cname,
+                    })
+
                 code.put_goto(code.error_label)
                 code.putln("}")
 
@@ -8643,21 +8648,18 @@ static CYTHON_INLINE %(type)s __Pyx_mod_%(type_name)s(%(type)s a, %(type)s b) {
 
 cdivision_warning_utility_code = UtilityCode(
 proto="""
-static int __Pyx_cdivision_warning(void); /* proto */
+static int __Pyx_cdivision_warning(const char *, int); /* proto */
 """,
 impl="""
-static int __Pyx_cdivision_warning(void) {
+static int __Pyx_cdivision_warning(const char *filename, int lineno) {
     return PyErr_WarnExplicit(PyExc_RuntimeWarning,
                               "division with oppositely signed operands, C and Python semantics differ",
-                              %(FILENAME)s,
-                              %(LINENO)s,
+                              filename,
+                              lineno,
                               __Pyx_MODULE_NAME,
                               NULL);
 }
-""" % {
-    'FILENAME': Naming.filename_cname,
-    'LINENO':  Naming.lineno_cname,
-})
+""")
 
 # from intobject.c
 division_overflow_test_code = UtilityCode(

Cython/Compiler/ModuleNode.py

@@ -1802,7 +1802,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
         if code.label_used(code.error_label):
             code.put_label(code.error_label)
             # This helps locate the offending name.
-            code.putln('__Pyx_AddTraceback("%s");' % self.full_module_name);
+            code.put_add_traceback(self.full_module_name)
         code.error_label = old_error_label
         code.putln("bad:")
         code.putln("return -1;")
@@ -1919,7 +1919,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
         for cname, type in code.funcstate.all_managed_temps():
             code.put_xdecref(cname, type)
         code.putln('if (%s) {' % env.module_cname)
-        code.putln('__Pyx_AddTraceback("init %s");' % env.qualified_name)
+        code.put_add_traceback("init %s" % env.qualified_name)
         env.use_utility_code(Nodes.traceback_utility_code)
         code.put_decref_clear(env.module_cname, py_object_type, nanny=False)
         code.putln('} else if (!PyErr_Occurred()) {')

Cython/Compiler/ParseTreeTransforms.py

@@ -638,6 +638,8 @@ class InterpretCompilerDirectives(CythonTransform, SkipDeclarations):
                                         'is not allowed in %s scope' % (directive, scope)))
             return False
         else:
+            if directive not in Options.directive_defaults:
+                error(pos, "Invalid directive: '%s'." % (directive,))
             return True
 
     # Set up processing and handle the cython: comments.
@@ -946,9 +948,9 @@ class InterpretCompilerDirectives(CythonTransform, SkipDeclarations):
                         PostParseError(node.pos, "Compiler directive with statements cannot contain 'as'"))
                 else:
                     name, value = directive
-                    if name == 'nogil':
+                    if name in ('nogil', 'gil'):
                         # special case: in pure mode, "with nogil" spells "with cython.nogil"
-                        node = Nodes.GILStatNode(node.pos, state = "nogil", body = node.body)
+                        node = Nodes.GILStatNode(node.pos, state = name, body = node.body)
                         return self.visit_Node(node)
                     if self.check_directive_scope(node.pos, name, 'with statement'):
                         directive_dict[name] = value
@@ -1361,6 +1363,18 @@ if VALUE is not None:
                 else:
                     error(type_node.pos, "Not a type")
         node.body.analyse_declarations(lenv)
+
+        if lenv.nogil and lenv.has_with_gil_block:
+            # Acquire the GIL for cleanup in 'nogil' functions, by wrapping
+            # the entire function body in try/finally.
+            # The corresponding release will be taken care of by
+            # Nodes.FuncDefNode.generate_function_definitions()
+            node.body = Nodes.NogilTryFinallyStatNode(
+                node.body.pos,
+                body = node.body,
+                finally_clause = Nodes.EnsureGILNode(node.body.pos),
+            )
+
         self.env_stack.append(lenv)
         self.visitchildren(node)
         self.env_stack.pop()
@@ -1532,6 +1546,7 @@ if VALUE is not None:
         # ---------------------------------------
         return property
 
+
 class AnalyseExpressionsTransform(CythonTransform):
 
     def visit_ModuleNode(self, node):
@@ -1553,6 +1568,7 @@ class AnalyseExpressionsTransform(CythonTransform):
         self.visitchildren(node)
         return node
 
+
 class ExpandInplaceOperators(EnvTransform):
 
     def visit_InPlaceAssignmentNode(self, node):
@@ -2016,10 +2032,18 @@ class GilCheck(VisitorTransform):
     Call `node.gil_check(env)` on each node to make sure we hold the
     GIL when we need it.  Raise an error when on Python operations
     inside a `nogil` environment.
+
+    Additionally, raise exceptions for closely nested with gil or with nogil
+    statements. The latter would abort Python.
     """
+
     def __call__(self, root):
         self.env_stack = [root.scope]
         self.nogil = False
+
+        # True for 'cdef func() nogil:' functions, as the GIL may be held while
+        # calling this function (thus contained 'nogil' blocks may be valid).
+        self.nogil_declarator_only = False
         return super(GilCheck, self).__call__(root)
 
     def visit_FuncDefNode(self, node):
@@ -2027,10 +2051,17 @@ class GilCheck(VisitorTransform):
         was_nogil = self.nogil
         self.nogil = node.local_scope.nogil
 
+        if self.nogil:
+            self.nogil_declarator_only = True
+
         if self.nogil and node.nogil_check:
             node.nogil_check(node.local_scope)
 
         self.visitchildren(node)
+
+        # This cannot be nested, so it doesn't need backup/restore
+        self.nogil_declarator_only = False
+
         self.env_stack.pop()
         self.nogil = was_nogil
         return node
@@ -2041,6 +2072,23 @@ class GilCheck(VisitorTransform):
 
         was_nogil = self.nogil
         self.nogil = (node.state == 'nogil')
+
+        if was_nogil == self.nogil and not self.nogil_declarator_only:
+            if not was_nogil:
+                error(node.pos, "Trying to acquire the GIL while it is "
+                                "already held.")
+            else:
+                error(node.pos, "Trying to release the GIL while it was "
+                                "previously released.")
+
+        if isinstance(node.finally_clause, Nodes.StatListNode):
+            # The finally clause of the GILStatNode is a GILExitNode,
+            # which is wrapped in a StatListNode. Just unpack that.
+            node.finally_clause, = node.finally_clause.stats
+
+        if node.state == 'gil':
+            self.seen_with_gil_statement = True
+
         self.visitchildren(node)
         self.nogil = was_nogil
         return node
@@ -2074,6 +2122,28 @@ class GilCheck(VisitorTransform):
         self.visitchildren(node)
         return node
 
+    def visit_TryFinallyStatNode(self, node):
+        """
+        Take care of try/finally statements in nogil code sections. The
+        'try' must contain a 'with gil:' statement somewhere.
+        """
+        if not self.nogil or isinstance(node, Nodes.GILStatNode):
+            return self.visit_Node(node)
+
+        node.nogil_check = None
+        node.is_try_finally_in_nogil = True
+
+        # First, visit the body and check for errors
+        self.seen_with_gil_statement = False
+        self.visitchildren(node.body)
+
+        if not self.seen_with_gil_statement:
+            error(node.pos, "Cannot use try/finally in nogil sections unless "
+                            "it contains a 'with gil' statement.")
+
+        self.visitchildren(node.finally_clause)
+        return node
+
     def visit_Node(self, node):
         if self.env_stack and self.nogil and node.nogil_check:
             node.nogil_check(self.env_stack[-1])

Cython/Compiler/Parsing.py

@@ -1580,7 +1580,7 @@ def p_with_statement(s):
 
 def p_with_items(s):
     pos = s.position()
-    if not s.in_python_file and s.sy == 'IDENT' and s.systring == 'nogil':
+    if not s.in_python_file and s.sy == 'IDENT' and s.systring in ('nogil', 'gil'):
         state = s.systring
         s.next()
         if s.sy == ',':

Cython/Compiler/Symtab.py

@@ -176,6 +176,7 @@ class Entry(object):
     buffer_aux = None
     prev_entry = None
     might_overflow = 0
+    in_with_gil_block = 0
 
     def __init__(self, name, cname, type, pos = None, init = None):
         self.name = name
@@ -1349,6 +1350,12 @@ class ModuleScope(Scope):
 
 class LocalScope(Scope):
 
+    # Does the function have a 'with gil:' block?
+    has_with_gil_block = False
+
+    # Transient attribute, used for symbol table variable declarations
+    _in_with_gil_block = False
+
     def __init__(self, name, outer_scope, parent_scope = None):
         if parent_scope is None:
             parent_scope = outer_scope
@@ -1383,6 +1390,8 @@ class LocalScope(Scope):
             entry.init = "0"
         entry.init_to_none = (type.is_pyobject or type.is_unspecified) and Options.init_local_none
         entry.is_local = 1
+
+        entry.in_with_gil_block = self._in_with_gil_block
         self.var_entries.append(entry)
         return entry
 

Cython/Includes/cpython/__init__.pxd

@@ -146,6 +146,7 @@ from cpython.method cimport *
 from cpython.weakref cimport *
 from cpython.getargs cimport *
 from cpython.pythread cimport *
+from cpython.pystate cimport *
 
 # Python <= 2.x
 from cpython.cobject cimport *

Cython/Includes/cpython/pystate.pxd

+# Thread and interpreter state structures and their interfaces
+
+from cpython.ref cimport PyObject
+
+cdef extern from "Python.h":
+
+    # We make these an opague types. If the user wants specific attributes,
+    # they can be declared manually.
+
+    ctypedef struct PyInterpreterState:
+        pass
+
+    ctypedef struct PyThreadState:
+        pass
+
+    ctypedef struct PyFrameObject:
+        pass
+
+    # This is not actually a struct, but make sure it can never be coerced to
+    # an int or used in arithmetic expressions
+    ctypedef struct PyGILState_STATE
+
+    # The type of the trace function registered using PyEval_SetProfile() and
+    # PyEval_SetTrace().
+    # Py_tracefunc return -1 when raising an exception, or 0 for success.
+    ctypedef int (*Py_tracefunc)(PyObject *, PyFrameObject *, int, PyObject *)
+
+    # The following values are used for 'what' for tracefunc functions
+    enum:
+        PyTrace_CALL
+        PyTrace_EXCEPTION
+        PyTrace_LINE
+        PyTrace_RETURN
+        PyTrace_C_CALL
+        PyTrace_C_EXCEPTION
+        PyTrace_C_RETURN
+
+
+    PyInterpreterState * PyInterpreterState_New()
+    void PyInterpreterState_Clear(PyInterpreterState *)
+    void PyInterpreterState_Delete(PyInterpreterState *)
+
+    PyThreadState * PyThreadState_New(PyInterpreterState *)
+    void PyThreadState_Clear(PyThreadState *)
+    void PyThreadState_Delete(PyThreadState *)
+
+    PyThreadState * PyThreadState_Get()
+    PyThreadState * PyThreadState_Swap(PyThreadState *)
+    PyObject * PyThreadState_GetDict()
+    int PyThreadState_SetAsyncExc(long, PyObject *)
+
+    # Ensure that the current thread is ready to call the Python
+    # C API, regardless of the current state of Python, or of its
+    # thread lock.  This may be called as many times as desired
+    # by a thread so long as each call is matched with a call to
+    # PyGILState_Release().  In general, other thread-state APIs may
+    # be used between _Ensure() and _Release() calls, so long as the
+    # thread-state is restored to its previous state before the Release().
+    # For example, normal use of the Py_BEGIN_ALLOW_THREADS/
+    # Py_END_ALLOW_THREADS macros are acceptable.
+
+    # The return value is an opaque "handle" to the thread state when
+    # PyGILState_Ensure() was called, and must be passed to
+    # PyGILState_Release() to ensure Python is left in the same state. Even
+    # though recursive calls are allowed, these handles can *not* be shared -
+    # each unique call to PyGILState_Ensure must save the handle for its
+    # call to PyGILState_Release.
+
+    # When the function returns, the current thread will hold the GIL.
+
+    # Failure is a fatal error.
+    PyGILState_STATE PyGILState_Ensure()
+
+    # Release any resources previously acquired.  After this call, Python's
+    # state will be the same as it was prior to the corresponding
+    # PyGILState_Ensure() call (but generally this state will be unknown to
+    # the caller, hence the use of the GILState API.)
+
+    # Every call to PyGILState_Ensure must be matched by a call to
+    # PyGILState_Release on the same thread.
+    void PyGILState_Release(PyGILState_STATE)
+
+    # Routines for advanced debuggers, requested by David Beazley.
+    # Don't use unless you know what you are doing!
+    PyInterpreterState * PyInterpreterState_Head()
+    PyInterpreterState * PyInterpreterState_Next(PyInterpreterState *)
+    PyThreadState * PyInterpreterState_ThreadHead(PyInterpreterState *)
+    PyThreadState * PyThreadState_Next(PyThreadState *)

Cython/Runtime/refnanny.pyx

 from cpython.ref cimport PyObject, Py_INCREF, Py_DECREF, Py_XDECREF
 from cpython.exc cimport PyErr_Fetch, PyErr_Restore
+from cpython.pystate cimport PyThreadState_Get
 
 
 loglevel = 0
@@ -80,6 +81,7 @@ cdef PyObject* SetupContext(char* funcname, int lineno, char* filename) except N
         return NULL
     cdef PyObject* type = NULL, *value = NULL, *tb = NULL
     cdef PyObject* result = NULL
+    PyThreadState_Get()
     PyErr_Fetch(&type, &value, &tb)
     try:
         ctx = Context(funcname, lineno, filename)
@@ -131,16 +133,19 @@ cdef void GIVEREF(PyObject* ctx, PyObject* p_obj, int lineno):
 
 cdef void INCREF(PyObject* ctx, PyObject* obj, int lineno):
     if obj is not NULL: Py_INCREF(<object>obj)
+    PyThreadState_Get()
     GOTREF(ctx, obj, lineno)
 
 cdef void DECREF(PyObject* ctx, PyObject* obj, int lineno):
     if GIVEREF_and_report(ctx, obj, lineno):
         if obj is not NULL: Py_DECREF(<object>obj)
+    PyThreadState_Get()
 
 cdef void FinishContext(PyObject** ctx):
     if ctx == NULL or ctx[0] == NULL: return
     cdef PyObject* type = NULL, *value = NULL, *tb = NULL
     cdef object errors = None
+    PyThreadState_Get()
     PyErr_Fetch(&type, &value, &tb)
     try:
         try:

Cython/Shadow.py

@@ -89,6 +89,7 @@ class _nogil(object):
         return exc_class is None
 
 nogil = _nogil()
+gil = _nogil()
 del _nogil
 
 # Emulated types

tests/errors/incorrectly_nested_gil_blocks.pyx

+# mode: error
+
+with gil:
+    pass
+
+with nogil:
+    with nogil:
+        pass
+
+cdef void without_gil() nogil:
+   # This is not an error, as 'func' *may* be called without the GIL, but it
+   # may also be held.
+    with nogil:
+        pass
+
+cdef void with_gil() with gil:
+    # This is an error, as the GIL is acquired already
+    with gil:
+        pass
+
+def func():
+    with gil:
+        pass
+
+_ERRORS = u'''
+3:5: Trying to acquire the GIL while it is already held.
+7:9: Trying to release the GIL while it was previously released.
+18:9: Trying to acquire the GIL while it is already held.
+22:9: Trying to acquire the GIL while it is already held.
+'''

tests/errors/nogil.pyx

@@ -158,6 +158,6 @@ _ERRORS = u"""
 60:17: Truth-testing Python object not allowed without gil
 62:8: For-loop using object bounds or target not allowed without gil
 64:8: Try-except statement not allowed without gil
-68:8: Try-finally statement not allowed without gil
+68:8: Cannot use try/finally in nogil sections unless it contains a 'with gil' statement.
 85:8: For-loop using object bounds or target not allowed without gil
 """

tests/run/parallel.pyx

@@ -24,4 +24,4 @@ def test_parallel():
 
     free(buf)
 
-#include "sequential_parallel.pyx"
+include "sequential_parallel.pyx"

tests/run/sequential_parallel.pyx

@@ -50,7 +50,7 @@ def test_propagation():
     >>> test_propagation()
     (9, 9, 9, 9, 450, 450)
     """
-    cdef int i, j, x, y
+    cdef int i = 0, j = 0, x = 0, y = 0
     cdef int sum1 = 0, sum2 = 0
 
     for i in prange(10, nogil=True):