bpo-40795: ctypes calls unraisablehook with an exception (GH-20452)

If ctypes fails to convert the result of a callback or if a ctypes
callback function raises an exception, sys.unraisablehook is now
called with an exception set. Previously, the error was logged into
stderr by PyErr_Print().
(cherry picked from commit 10228bad0452d94e66c964b625a0b61befa08e59)

Co-authored-by: Victor Stinner <vstinner@python.org>

6 files changed, 79 insertions, 41 deletions

diff --git a/Lib/ctypes/test/test_callbacks.py b/Lib/ctypes/test/test_callbacks.py
index 937a06d..d8e9c5a 100644
--- a/Lib/ctypes/test/test_callbacks.py
+++ b/Lib/ctypes/test/test_callbacks.py
@@ -1,5 +1,7 @@
 import functools
 import unittest
+from test import support
+
 from ctypes import *
 from ctypes.test import need_symbol
 import _ctypes_test
@@ -301,8 +303,22 @@ class SampleCallbacksTestCase(unittest.TestCase):
         with self.assertRaises(ArgumentError):
             cb(*args2)
 
+    def test_convert_result_error(self):
+        def func():
+            return ("tuple",)
+
+        proto = CFUNCTYPE(c_int)
+        ctypes_func = proto(func)
+        with support.catch_unraisable_exception() as cm:
+            # don't test the result since it is an uninitialized value
+            result = ctypes_func()
+
+            self.assertIsInstance(cm.unraisable.exc_value, TypeError)
+            self.assertEqual(cm.unraisable.err_msg,
+                             "Exception ignored on converting result "
+                             "of ctypes callback function")
+            self.assertIs(cm.unraisable.object, func)
 
-################################################################
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/Lib/ctypes/test/test_random_things.py b/Lib/ctypes/test/test_random_things.py
index ee5b212..2988e27 100644
--- a/Lib/ctypes/test/test_random_things.py
+++ b/Lib/ctypes/test/test_random_things.py
@@ -1,5 +1,9 @@
 from ctypes import *
-import unittest, sys
+import contextlib
+from test import support
+import unittest
+import sys
+
 
 def callback_func(arg):
     42 / arg
@@ -34,41 +38,40 @@ class CallbackTracbackTestCase(unittest.TestCase):
     # created, then a full traceback printed.  When SystemExit is
     # raised in a callback function, the interpreter exits.
 
-    def capture_stderr(self, func, *args, **kw):
-        # helper - call function 'func', and return the captured stderr
-        import io
-        old_stderr = sys.stderr
-        logger = sys.stderr = io.StringIO()
-        try:
-            func(*args, **kw)
-        finally:
-            sys.stderr = old_stderr
-        return logger.getvalue()
+    @contextlib.contextmanager
+    def expect_unraisable(self, exc_type, exc_msg=None):
+        with support.catch_unraisable_exception() as cm:
+            yield
+
+            self.assertIsInstance(cm.unraisable.exc_value, exc_type)
+            if exc_msg is not None:
+                self.assertEqual(str(cm.unraisable.exc_value), exc_msg)
+            self.assertEqual(cm.unraisable.err_msg,
+                             "Exception ignored on calling ctypes "
+                             "callback function")
+            self.assertIs(cm.unraisable.object, callback_func)
 
     def test_ValueError(self):
         cb = CFUNCTYPE(c_int, c_int)(callback_func)
-        out = self.capture_stderr(cb, 42)
-        self.assertEqual(out.splitlines()[-1],
-                             "ValueError: 42")
+        with self.expect_unraisable(ValueError, '42'):
+            cb(42)
 
     def test_IntegerDivisionError(self):
         cb = CFUNCTYPE(c_int, c_int)(callback_func)
-        out = self.capture_stderr(cb, 0)
-        self.assertEqual(out.splitlines()[-1][:19],
-                             "ZeroDivisionError: ")
+        with self.expect_unraisable(ZeroDivisionError):
+            cb(0)
 
     def test_FloatDivisionError(self):
         cb = CFUNCTYPE(c_int, c_double)(callback_func)
-        out = self.capture_stderr(cb, 0.0)
-        self.assertEqual(out.splitlines()[-1][:19],
-                             "ZeroDivisionError: ")
+        with self.expect_unraisable(ZeroDivisionError):
+            cb(0.0)
 
     def test_TypeErrorDivisionError(self):
         cb = CFUNCTYPE(c_int, c_char_p)(callback_func)
-        out = self.capture_stderr(cb, b"spam")
-        self.assertEqual(out.splitlines()[-1],
-                             "TypeError: "
-                             "unsupported operand type(s) for /: 'int' and 'bytes'")
+        err_msg = "unsupported operand type(s) for /: 'int' and 'bytes'"
+        with self.expect_unraisable(TypeError, err_msg):
+            cb(b"spam")
+
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/Lib/ctypes/test/test_unaligned_structures.py b/Lib/ctypes/test/test_unaligned_structures.py
index bcacfc8..ee7fb45 100644
--- a/Lib/ctypes/test/test_unaligned_structures.py
+++ b/Lib/ctypes/test/test_unaligned_structures.py
@@ -27,7 +27,6 @@ for typ in [c_short, c_int, c_long, c_longlong,
 class TestStructures(unittest.TestCase):
     def test_native(self):
         for typ in structures:
-##            print typ.value
             self.assertEqual(typ.value.offset, 1)
             o = typ()
             o.value = 4
@@ -35,7 +34,6 @@ class TestStructures(unittest.TestCase):
 
     def test_swapped(self):
         for typ in byteswapped_structures:
-##            print >> sys.stderr, typ.value
             self.assertEqual(typ.value.offset, 1)
             o = typ()
             o.value = 4
diff --git a/Misc/NEWS.d/next/Library/2020-05-27-17-00-18.bpo-40795.eZSnHA.rst b/Misc/NEWS.d/next/Library/2020-05-27-17-00-18.bpo-40795.eZSnHA.rst
new file mode 100644
index 0000000..dd02fb0
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-05-27-17-00-18.bpo-40795.eZSnHA.rst
@@ -0,0 +1,4 @@
+:mod:`ctypes` module: If ctypes fails to convert the result of a callback or
+if a ctypes callback function raises an exception, sys.unraisablehook is now
+called with an exception set. Previously, the error was logged into stderr
+by :c:func:`PyErr_Print`.
diff --git a/Modules/_ctypes/callbacks.c b/Modules/_ctypes/callbacks.c
index d2d9a65..2a364d6 100644
--- a/Modules/_ctypes/callbacks.c
+++ b/Modules/_ctypes/callbacks.c
@@ -213,9 +213,6 @@ static void _CallPythonObject(void *mem,
         pArgs++;
     }
 
-#define CHECK(what, x) \
-if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyErr_Print()
-
     if (flags & (FUNCFLAG_USE_ERRNO | FUNCFLAG_USE_LASTERROR)) {
         error_object = _ctypes_get_errobj(&space);
         if (error_object == NULL)
@@ -235,7 +232,10 @@ if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyEr
     }
 
     result = PyObject_CallObject(callable, arglist);
-    CHECK("'calling callback function'", result);
+    if (result == NULL) {
+        _PyErr_WriteUnraisableMsg("on calling ctypes callback function",
+                                  callable);
+    }
 
 #ifdef MS_WIN32
     if (flags & FUNCFLAG_USE_LASTERROR) {
@@ -251,16 +251,17 @@ if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyEr
     }
     Py_XDECREF(error_object);
 
-    if ((restype != &ffi_type_void) && result) {
-        PyObject *keep;
+    if (restype != &ffi_type_void && result) {
         assert(setfunc);
+
 #ifdef WORDS_BIGENDIAN
-        /* See the corresponding code in callproc.c, around line 961 */
-        if (restype->type != FFI_TYPE_FLOAT && restype->size < sizeof(ffi_arg))
+        /* See the corresponding code in _ctypes_callproc():
+           in callproc.c, around line 1219. */
+        if (restype->type != FFI_TYPE_FLOAT && restype->size < sizeof(ffi_arg)) {
             mem = (char *)mem + sizeof(ffi_arg) - restype->size;
+        }
 #endif
-        keep = setfunc(mem, result, 0);
-        CHECK("'converting callback result'", keep);
+
         /* keep is an object we have to keep alive so that the result
            stays valid.  If there is no such object, the setfunc will
            have returned Py_None.
@@ -270,18 +271,32 @@ if (x == NULL) _PyTraceback_Add(what, "_ctypes/callbacks.c", __LINE__ - 1), PyEr
            be the result.  EXCEPT when restype is py_object - Python
            itself knows how to manage the refcount of these objects.
         */
-        if (keep == NULL) /* Could not convert callback result. */
-            PyErr_WriteUnraisable(callable);
-        else if (keep == Py_None) /* Nothing to keep */
+        PyObject *keep = setfunc(mem, result, 0);
+
+        if (keep == NULL) {
+            /* Could not convert callback result. */
+            _PyErr_WriteUnraisableMsg("on converting result "
+                                      "of ctypes callback function",
+                                      callable);
+        }
+        else if (keep == Py_None) {
+            /* Nothing to keep */
             Py_DECREF(keep);
+        }
         else if (setfunc != _ctypes_get_fielddesc("O")->setfunc) {
             if (-1 == PyErr_WarnEx(PyExc_RuntimeWarning,
                                    "memory leak in callback function.",
                                    1))
-                PyErr_WriteUnraisable(callable);
+            {
+                _PyErr_WriteUnraisableMsg("on converting result "
+                                          "of ctypes callback function",
+                                          callable);
+            }
         }
     }
+
     Py_XDECREF(result);
+
   Done:
     Py_XDECREF(arglist);
     PyGILState_Release(state);
diff --git a/Modules/_ctypes/callproc.c b/Modules/_ctypes/callproc.c
index e0a110d..55fc226 100644
--- a/Modules/_ctypes/callproc.c
+++ b/Modules/_ctypes/callproc.c
@@ -1232,7 +1232,9 @@ PyObject *_ctypes_callproc(PPROC pProc,
     if (rtype->type != FFI_TYPE_FLOAT
         && rtype->type != FFI_TYPE_STRUCT
         && rtype->size < sizeof(ffi_arg))
+    {
         resbuf = (char *)resbuf + sizeof(ffi_arg) - rtype->size;
+    }
 #endif
 
 #ifdef MS_WIN32