Enclose the entire function body of cdef nogil functions in try/finally + tests

Cython/Compiler/Code.py

@@ -1384,7 +1384,7 @@ class CCodeWriter(object):
 
     # GIL methods
 
-    def put_ensure_gil(self):
+    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
@@ -1396,7 +1396,9 @@ class CCodeWriter(object):
         self.globalstate.use_utility_code(Nodes.force_init_threads_utility_code)
 
         self.putln("#ifdef WITH_THREAD")
-        self.putln("PyGILState_STATE _save = PyGILState_Ensure();")
+        if declare_gilstate:
+            self.put("PyGILState_STATE ")
+        self.putln("_save = PyGILState_Ensure();")
         self.putln("#endif")
 
     def put_release_ensured_gil(self):

Cython/Compiler/Nodes.py

@@ -1498,7 +1498,6 @@ class FuncDefNode(StatNode, BlockNode):
             if buffers_present or is_getbuffer_slot:
                 code.put_goto(code.return_from_error_cleanup_label)
 
-
         # ----- Non-error return cleanup
         code.put_label(code.return_label)
         for entry in lenv.buffer_entries:
@@ -1550,17 +1549,8 @@ class FuncDefNode(StatNode, BlockNode):
         if not lenv.nogil:
             # GIL holding funcion
             code.put_finish_refcount_context()
-        elif acquire_gil_for_var_decls_only:
-            # 'nogil' function with 'with gil:' block, tear down refnanny
-            code.putln("#if CYTHON_REFNANNY")
-            code.begin_block()
-            code.put_ensure_gil()
-            code.put_finish_refcount_context()
-            code.put_release_ensured_gil()
-            code.end_block()
-            code.putln("#endif")
 
-        if acquire_gil:
+        if acquire_gil or acquire_gil_for_var_decls_only:
             code.put_release_ensured_gil()
 
         if not self.return_type.is_void:
@@ -5607,6 +5597,15 @@ class GILExitNode(StatNode):
             code.put_acquire_gil()
 
 
+class EnsureGILNode(GILExitNode):
+    """
+    Ensure the GIL in nogil functions for cleanup before returning.
+    """
+
+    def generate_execution_code(self, code):
+        code.put_ensure_gil(declare_gilstate=False)
+
+
 class CImportStatNode(StatNode):
     #  cimport statement
     #

Cython/Compiler/ParseTreeTransforms.py

@@ -1363,6 +1363,19 @@ 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. The
+            # corresponding release will be taken care of by
+            # Nodes.FuncDefNode.generate_function_definitions()
+            node.body = Nodes.TryFinallyStatNode(
+                node.body.pos,
+                body = node.body,
+                finally_clause = Nodes.EnsureGILNode(node.body.pos),
+                preserve_exception = False,
+                nogil_check = None,
+            )
+
         self.env_stack.append(lenv)
         self.visitchildren(node)
         self.env_stack.pop()

tests/run/with_gil.pyx

 """
-Most of these functions are 'cdef' functions, so we need to test using 'def'
-test wrappers.
+Test the 'with gil:' statement.
 """
 
-from libc.stdio cimport printf
+cimport cython
+
+#from libc.stdio cimport printf, puts
 from cpython.ref cimport PyObject, Py_INCREF
 
 import sys
 
-try:
-    import StringIO
-except ImportError:
-    import io as StringIO
 
-def simple_func():
+def redirect_stderr(func, *args, **kwargs):
+    """
+    Helper function that redirects stderr to stdout for doctest.
+    """
+    stderr, sys.stderr = sys.stderr, sys.stdout
+    func(*args, **kwargs)
+    sys.stderr = stderr
+
+cdef void puts(char *string) with gil:
+    """
+    We need this for doctest, used from nogil sections.
+    """
+    print string
+
+
+# Start with some normal Python functions
+
+def test_simple():
     """
-    >>> simple_func()
+    >>> test_simple()
     ['spam', 'ham']
-    ('star', 'twinkle')
     """
     with nogil:
         with gil:
             print ['spam', 'ham']
-        cdef_simple_func()
 
-cdef void cdef_simple_func() nogil:
-    with gil:
-        print ('star', 'twinkle')
+def test_nested_gil_blocks():
+    """
+    >>> test_nested_gil_blocks()
+    entered outer nogil section
+    entered outer gil section
+    entered inner nogil section
+    entered inner gil section
+    leaving inner gil section
+    leaving inner nogil section
+    leaving outer gil section
+    leaving outer nogil section
+    """
+
+    with nogil:
+        puts("entered outer nogil section")
+
+        with gil:
+            print 'entered outer gil section'
+
+            with nogil:
+                puts("entered inner nogil section")
+                with gil:
+                    print 'entered inner gil section'
+                    print 'leaving inner gil section'
+                puts("leaving inner nogil section")
+
+            print "leaving outer gil section"
+        puts("leaving outer nogil section")
+
+def test_propagate_exception():
+    """
+    >>> test_propagate_exception()
+    Traceback (most recent call last):
+        ...
+    Exception: This exception propagates!
+    """
+    # Note, doctest doesn't support both output and exceptions
+    with nogil:
+        with gil:
+            raise Exception("This exception propagates!")
+
+def test_catch_exception():
+    """
+    >>> test_catch_exception()
+    This is executed
+    Exception value
+    This is also executed
+    """
+    try:
+        with nogil:
+            with gil:
+                print "This is executed"
+                raise Exception("Exception value")
+                print "This is not executed"
+            puts("This is also not executed")
+    except Exception, e:
+        print e
+    print "This is also executed"
+
+def test_try_finally_and_outer_except():
+    """
+    >>> test_try_finally_and_outer_except()
+    First finally clause
+    Second finally clause...
+    Caught: Some Exception
+    End of function
+    """
+    try:
+
+        with nogil:
+            with gil:
+                try:
+                    with nogil:
+                        with gil:
+                            try:
+                                raise Exception("Some Exception")
+                            finally:
+                                puts("First finally clause")
+                finally:
+                    puts("Second finally clause...")
+            puts("This is not executed")
+
+    except Exception, e:
+        print "Caught:", e
+
+    print "End of function"
+
+def test_declared_variables():
+    """
+    >>> test_declared_variables()
+    None
+    None
+    ['s', 'p', 'a', 'm']
+    ['s', 'p', 'a', 'm']
+    """
+    cdef object somevar
+
+    print somevar
+
+    with nogil:
+        with gil:
+            print somevar
+            somevar = list("spam")
+            print somevar
+
+    print somevar
 
-def with_gil():
+def test_undeclared_variables():
     """
-    >>> with_gil()
+    >>> test_undeclared_variables()
+    None
     None
-    {'spam': 'ham'}
+    ['s', 'p', 'a', 'm']
+    ['s', 'p', 'a', 'm']
+    """
+    print somevar
+    with nogil:
+        with gil:
+            print somevar
+            somevar = list("spam")
+            print somevar
+
+    print somevar
+
+def test_loops_and_boxing():
+    """
+    >>> test_loops_and_boxing()
+    spamham
+    h
+    a
+    m
+    done looping
+    """
+    cdef char c, *string = "spamham"
+
+    with nogil:
+        with gil:
+            print string
+            for c in string[4:]:
+                print "%c" % c
+            else:
+                print "done looping"
+
+cdef class SomeExtClass(object):
+    cdef int some_attribute
+
+@cython.infer_types(True)
+def test_infer_types():
+    """
+    >>> test_infer_types()
+    10
+    """
+    with nogil:
+        with gil:
+            obj = SomeExtClass()
+            obj.some_attribute = 10
+
+    print obj.some_attribute
+
+cpdef test_cpdef():
+    """
+    >>> test_cpdef()
+    Seems to work!
+    Or does it? ...
     """
-    print x
     with nogil:
         with gil:
-            x = dict(spam='ham')
-    print x
+            print "Seems to work!"
+        puts("Or does it? ...")
+
 
+# Now test some cdef functions with different return types
 
-cdef void without_gil() nogil:
+cdef void void_nogil_ignore_exception() nogil:
     with gil:
-        x = list(('foo', 'bar'))
         raise NameError
 
+    puts("unreachable")
     with gil:
         print "unreachable"
 
-def test_without_gil():
+def test_void_nogil_ignore_exception():
     """
-    >>> test_without_gil()
-    Exception NameError in 'with_gil.without_gil' ignored
+    >>> redirect_stderr(test_void_nogil_ignore_exception)
+    Exception NameError in 'with_gil.void_nogil_ignore_exception' ignored
+    Exception NameError in 'with_gil.void_nogil_ignore_exception' ignored
     """
-    # Doctest doesn't capture-and-match stderr
-    stderr, sys.stderr = sys.stderr, StringIO.StringIO()
-    without_gil()
-    sys.stdout.write(sys.stderr.getvalue())
-    sys.stderr = stderr
+    void_nogil_ignore_exception()
+    with nogil:
+        void_nogil_ignore_exception()
+
 
 cdef PyObject *nogil_propagate_exception() nogil except NULL:
     with nogil:
@@ -75,3 +243,17 @@ def test_nogil_propagate_exception():
     """
     nogil_propagate_exception()
 
+
+cdef with_gil_raise() with gil:
+    raise Exception("This exception propagates!")
+
+def test_release_gil_call_gil_func():
+    """
+    >>> test_release_gil_call_gil_func()
+    Traceback (most recent call last):
+        ...
+    Exception: This exception propagates!
+    """
+    with nogil:
+        with gil:
+            with_gil_raise()