Merge pull request #286 from python-greenlet/issue285

Extend the unhandled C++ exception tests to Windows

src/greenlet/greenlet.cpp

@@ -1150,7 +1150,23 @@ UserGreenlet::inner_bootstrap(OwnedGreenlet& origin_greenlet, OwnedObject& run)
     else {
         /* call g.run(*args, **kwargs) */
         // This could result in further switches
-        result = run.PyCall(args.args(), args.kwargs());
+        try {
+            result = run.PyCall(args.args(), args.kwargs());
+        }
+        catch(...) {
+            // Unhandled exception! If we don't catch this here, most
+            // platforms will just abort() the process. But on 64-bit
+            // Windows with older versions of the C runtime, this can
+            // actually corrupt memory and just return. We see this
+            // when compiling with the Windows 7.0 SDK targeting
+            // Windows Server 2008, but not when using the Appveyor
+            // Visual Studio 2019 image. So this currently only
+            // affects Python 2.7 on Windows 64. That is, the tests
+            // pass and the runtime aborts. But if we catch it and try
+            // to continue with a Python error, then all Windows 64
+            // bit platforms corrupt memory. So all we can do is abort.
+            abort();
+        }
     }
     args.CLEAR();
     run.CLEAR();

src/greenlet/tests/_test_extension_cpp.cpp

@@ -70,12 +70,76 @@ test_exception_switch(PyObject* UNUSED(self), PyObject* args)
     return p_test_exception_switch_recurse(depth, depth);
 }
 
+
+static PyObject*
+py_test_exception_throw(PyObject* self, PyObject* args)
+{
+    if (!PyArg_ParseTuple(args, ""))
+        return NULL;
+    p_test_exception_throw(0);
+    PyErr_SetString(PyExc_AssertionError, "unreachable code running after throw");
+    return NULL;
+}
+
+
+/* test_exception_switch_and_do_in_g2(g2func)
+ * - creates new greenlet g2 to run g2func
+ * - switches to g2 inside try/catch block
+ * - verifies that no exception has been caught
+ *
+ * it is used together with test_exception_throw to verify that unhandled
+ * exceptions thrown in one greenlet do not propagate to other greenlet nor
+ * segfault the process.
+ */
+static PyObject*
+test_exception_switch_and_do_in_g2(PyObject* self, PyObject* args)
+{
+    PyObject* g2func = NULL;
+    PyObject* result = NULL;
+
+    if (!PyArg_ParseTuple(args, "O", &g2func))
+        return NULL;
+    PyGreenlet* g2 = PyGreenlet_New(g2func, NULL);
+    if (!g2) {
+        return NULL;
+    }
+
+    try {
+        result = PyGreenlet_Switch(g2, NULL, NULL);
+        if (!result) {
+            return NULL;
+        }
+    }
+    catch (...) {
+        /* if we are here the memory can be already corrupted and the program
+         * might crash before below py-level exception might become printed.
+         * -> print something to stderr to make it clear that we had entered
+         *    this catch block.
+         */
+        fprintf(stderr, "C++ exception unexpectedly caught in g1\n");
+        PyErr_SetString(PyExc_AssertionError, "C++ exception unexpectedly caught in g1");
+    }
+
+    Py_XDECREF(result);
+    Py_RETURN_NONE;
+}
+
 static PyMethodDef test_methods[] = {
     {"test_exception_switch",
      (PyCFunction)&test_exception_switch,
      METH_VARARGS,
      "Switches to parent twice, to test exception handling and greenlet "
      "switching."},
+    {"test_exception_switch_and_do_in_g2",
+     (PyCFunction)&test_exception_switch_and_do_in_g2,
+     METH_VARARGS,
+     "Creates new greenlet g2 to run g2func and switches to it inside try/catch "
+     "block. Used together with test_exception_throw to verify that unhandled "
+     "C++ exceptions thrown in a greenlet doe not corrupt memory."},
+    {"test_exception_throw",
+     (PyCFunction)&py_test_exception_throw,
+     METH_VARARGS,
+     "Throws C++ exception. Calling this function directly should abort the process."},
     {NULL, NULL, 0, NULL}};
 
 #if PY_MAJOR_VERSION >= 3

src/greenlet/tests/test_cpp.py

@@ -1,12 +1,23 @@
 from __future__ import print_function
 from __future__ import absolute_import
 
-
+import signal
+from multiprocessing import Process
 
 import greenlet
 from . import _test_extension_cpp
 from . import TestCase
 
+def run_unhandled_exception_in_greenlet_aborts():
+    # This is used in multiprocessing.Process and must be picklable
+    # so it needs to be global.
+    def _():
+        _test_extension_cpp.test_exception_switch_and_do_in_g2(
+            _test_extension_cpp.test_exception_throw
+        )
+    g1 = greenlet.greenlet(_)
+    g1.switch()
+
 class CPPTests(TestCase):
     def test_exception_switch(self):
         greenlets = []
@@ -16,3 +27,39 @@ def test_exception_switch(self):
             greenlets.append(g)
         for i, g in enumerate(greenlets):
             self.assertEqual(g.switch(), i)
+
+    def _do_test_unhandled_exception(self, target):
+        # TODO: On some versions of Python with some settings, this
+        # spews a lot of garbage to stderr. It would be nice to capture and ignore that.
+        import sys
+        WIN = sys.platform.startswith("win")
+
+        p = Process(target=target)
+        p.start()
+        p.join(10)
+        # The child should be aborted in an unusual way. On POSIX
+        # platforms, this is done with abort() and signal.SIGABRT,
+        # which is reflected in a negative return value; however, on
+        # Windows, even though we observe the child print "Fatal
+        # Python error: Aborted" and in older versions of the C
+        # runtime "This application has requested the Runtime to
+        # terminate it in an unusual way," it always has an exit code
+        # of 3. This is interesting because 3 is the error code for
+        # ERROR_PATH_NOT_FOUND; BUT: the C runtime abort() function
+        # also uses this code.
+        #
+        # See
+        # https://devblogs.microsoft.com/oldnewthing/20110519-00/?p=10623
+        # and
+        # https://docs.microsoft.com/en-us/previous-versions/k089yyh0(v=vs.140)?redirectedfrom=MSDN
+        expected_exit = -signal.SIGABRT if not WIN else 3
+        self.assertEqual(p.exitcode, expected_exit)
+
+    def test_unhandled_exception_aborts(self):
+        # verify that plain unhandled throw aborts
+        self._do_test_unhandled_exception(_test_extension_cpp.test_exception_throw)
+
+
+    def test_unhandled_exception_in_greenlet_aborts(self):
+        # verify that unhandled throw called in greenlet aborts too
+        self._do_test_unhandled_exception(run_unhandled_exception_in_greenlet_aborts)