Fix crash when an exception occurs during module init and the stale module...

Fix crash when an exception occurs during module init and the stale module gets used later on (after import failure).
This is fixed by no longer cleaning up the global references to module object and module dict, which obviously has the drawback of leaking memory.

CHANGES.rst

@@ -2,6 +2,17 @@
 Cython Changelog
 ================
 
+Latest
+======
+
+Bugs fixed
+----------
+
+* Fatal exceptions in global module init code could lead to crashes
+  if the already created module was used later on (e.g. through a
+  stale reference in sys.modules or elsewhere).
+
+
 0.21 (2014-09-10)
 =================
 

Cython/Compiler/ModuleNode.py

@@ -2145,9 +2145,13 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
         code.putln('if (%s) {' % env.module_dict_cname)
         code.put_add_traceback("init %s" % env.qualified_name)
         code.globalstate.use_utility_code(Nodes.traceback_utility_code)
-        code.put_decref_clear(env.module_dict_cname, py_object_type, nanny=False)
+        # Module reference and module dict are in global variables which might still be needed
+        # for cleanup, atexit code, etc., so leaking is better than crashing.
+        # At least clearing the module dict here might be a good idea, but could still break
+        # user code in atexit or other global registries.
+        ##code.put_decref_clear(env.module_dict_cname, py_object_type, nanny=False)
         code.putln('}')
-        code.put_decref_clear(env.module_cname, py_object_type, nanny=False)
+        ##code.put_decref_clear(env.module_cname, py_object_type, nanny=False)
         code.putln('} else if (!PyErr_Occurred()) {')
         code.putln('PyErr_SetString(PyExc_ImportError, "init %s");' % env.qualified_name)
         code.putln('}')

tests/run/module_init_error.srctree

+PYTHON setup.py build_ext --inplace
+PYTHON -c "import test_fail_in_init; test_fail_in_init.try_import()"
+PYTHON -c "import test_fail_in_init_after_atexit; test_fail_in_init_after_atexit.try_import()"
+
+######## setup.py ########
+
+from Cython.Build.Dependencies import cythonize
+from distutils.core import setup
+
+setup(
+    ext_modules = cythonize("fail_in_init*.pyx")
+)
+
+######## test_fail_in_init.py ########
+
+import sys
+
+def try_import():
+    try:
+        import fail_in_init
+    except ValueError:
+        pass
+    else:
+        raise RuntimeError("expected ValueError from import")
+
+    if sys.version_info >= (3, 3):
+        assert 'fail_in_init' not in sys.modules
+    elif 'fail_in_init' in sys.modules:
+        try:
+            sys.modules['fail_in_init'].fail()
+        except AttributeError:
+            pass  # this is "ok enough"
+        except ValueError:
+            pass  # this is what we had expected
+        else:
+            raise RuntimeError("expected ValueError from call through sys.modules")
+
+######## fail_in_init.pyx ########
+
+def fail():
+    raise ValueError("kaputt")
+
+fail()
+
+######## test_fail_in_init_after_atexit.py ########
+
+def try_import():
+    try:
+        import fail_in_init_after_atexit
+    except ValueError:
+        pass
+    else:
+        raise RuntimeError("expected ValueError from import")
+
+######## fail_in_init_after_atexit.pyx ########
+
+X = 5
+
+def callback():
+    try:
+        print(X)
+    except NameError:
+        pass  # NameError is acceptable, a crash is not
+
+import atexit
+atexit.register(callback)
+
+def fail():
+    raise ValueError("holla!")
+
+fail()