Simplify Interpreter.call() for now.

python · ericsnowcurrently · Feb 28, 2024 · Feb 13, 2024 · Feb 13, 2024 · Feb 13, 2024
commit ca1dadf91b5fed2cd4016ea031e6b87e84a3da09
diff --git a/Lib/test/support/interpreters/__init__.py b/Lib/test/support/interpreters/__init__.py
@@ -14,7 +14,8 @@
 __all__ = [
     'get_current', 'get_main', 'create', 'list_all', 'is_shareable',
     'Interpreter',
-    'InterpreterError', 'InterpreterNotFoundError', 'ExecFailure',
+    'InterpreterError', 'InterpreterNotFoundError',
+    'ExecFailure', 'CallFailure',
     'NotShareableError',
     'create_queue', 'Queue', 'QueueEmpty', 'QueueFull',
 ]
@@ -43,7 +44,7 @@ def __getattr__(name):
 {formatted}
 """.strip()
 
-class ExecFailure(RuntimeError):
+class _ExecFailure(RuntimeError):
 
     def __init__(self, excinfo):
         msg = excinfo.formatted
@@ -67,6 +68,14 @@ def __str__(self):
             )
 
 
+class ExecFailure(_ExecFailure):
+    """Raised from Interpreter.exec() for unhandled exceptions."""
+
+
+class CallFailure(_ExecFailure):
+    """Raised from Interpreter.call() for unhandled exceptions."""
+
+
 def create():
     """Return a new (idle) Python interpreter."""
    id = _interpreters.create(isolated=True)
@@ -180,60 +189,33 @@ def exec(self, code, /):
         if excinfo is not None:
             raise ExecFailure(excinfo)
 
-    def call(self, callable, /, args=None, kwargs=None):
+    def call(self, callable, /):
         """Call the object in the interpreter with given args/kwargs.
 
-        Return the function's return value.  If it raises an exception,
-        raise it in the calling interpreter.  This contrasts with
-        Interpreter.exec(), which discards the return value and only
-        propagates the exception as ExecFailure.
+        Only functions that take no arguments and have no closure
+        are supported.
 
-        Unlike Interpreter.exec() and prepare_main(), all objects are
-        supported, at the expense of some performance.
+        The return value is discarded.
+
+        If the callable raises an exception then the error display
+        (including full traceback) is send back between the interpreters
+        and a CallFailedError is raised, much like what happens with
+        Interpreter.exec().
         """
-        pickled_callable = pickle.dumps(callable)
-        pickled_args = pickle.dumps(args)
-        pickled_kwargs = pickle.dumps(kwargs)
-
-        results = create_queue(sharedonly=False)
-        self.prepare_main(_call_results=results)
-        self.exec(f"""
-            def _call_impl():
-                try:
-                    import pickle
-                    callable = pickle.loads({pickled_callable!r})
-                    if {pickled_args!r} is None:
-                        args = ()
-                    else:
-                        args = pickle.loads({pickled_args!r})
-                    if {pickled_kwargs!r} is None:
-                        kwargs = {}
-                    else:
-                        kwargs = pickle.loads({pickled_kwargs!r})
-
-                    res = callable(*args, **kwargs)
-                except Exception as exc:
-                    res = pickle.dumps((None, exc))
-                else:
-                    res = pickle.dumps((res, None))
-                _call_results.put(res)
-            _call_impl()
-            del _call_impl
-            del _call_results
-            """)
-        res, exc = results.get()
-        if exc is None:
-            raise exc
-        else:
-            return res
+        # XXX Support args and kwargs.
+        # XXX Support arbitrary callables.
+        # XXX Support returning the return value (e.g. via pickle).
+        excinfo = _interpreters.call(self._id, callable)
+        if excinfo is not None:
+            raise CallFailure(excinfo)
 
-    def call_in_thread(self, callable, /, args=None, kwargs=None):
+    def call_in_thread(self, callable, /):
         """Return a new thread that calls the object in the interpreter.
 
         The return value and any raised exception are discarded.
         """
         def task():
-            self.call(callable, args, kwargs)
+            self.call(callable)
         t = threading.Thread(target=task)
         t.start()
         return t
@@ -746,7 +746,7 @@ def call_func_complex(op, /, value=None, *args, exc=None, **kwargs):
         class Eggs(Spam):
             pass
         return Eggs(value)
-    else if not isinstance(op, str):
+    elif not isinstance(op, str):
         raise TypeError(op)
     else:
         raise NotImplementedError(op)
@@ -787,61 +787,43 @@ class TestInterpreterCall(TestBase):
     def test_call(self):
         interp = interpreters.create()
 
-        for i, ((callable, args, kwargs), expected) in enumerate([
-            ((call_func_noop, (), {}),
-             None),
-            ((call_func_return_shareable, (), {}),
-             (1, None)),
-            ((call_func_return_not_shareable, (), {}),
-             [1, 2, 3]),
-            ((call_func_ident, ('spamspamspam',), {}),
-             'spamspamspam'),
-            ((get_call_func_closure, (42,), {}),
-             ...),
-            ((get_call_func_closure(42), (), {}),
-             42),
-            ((Spam.noop, (), {}),
-             None),
-            ((Spam.from_values, (), {}),
-             None),
-            ((Spam.from_values, (1, 2, 3), {}),
-             Spam((1, 2, 3)),
-            ((Spam, ('???'), {}),
-             Spam('???')),
-            ((Spam(101), (), {}),
-             101),
-            ((Spam(10101).run, (), {}),
-             10101),
-            ((call_func_complex, ('ident', 'spam'), {}),
-             'spam'),
-            ((call_func_complex, ('full-ident', 'spam'), {}),
-             ('spam', (), {})),
-            ((call_func_complex, ('full-ident', 'spam', 'ham'), {'eggs': '!!!'}),
-                ('spam', ('ham',), {'eggs': '!!!'})),
-            ((call_func_complex, ('globals',), {}),
-             'test.test_interpreters.test_api'),
-            ((call_func_complex, ('interpid',), {}),
-             interp.id),
-            ((call_func_complex, ('closure',), {'value': '~~~'}),
-             '~~~'),
-            ((call_func_complex, ('custom', 'spam!'), {}),
-             Spam('spam!')),
-            ((call_func_complex, ('custom-inner', 'eggs!'), {}),
-             ...),
+        for i, (callable, args, kwargs) in enumerate([
+            (call_func_noop, (), {}),
+            (call_func_return_shareable, (), {}),
+            (call_func_return_not_shareable, (), {}),
+            (Spam.noop, (), {}),
         ]):
             with self.subTest(f'success case #{i+1}'):
-                res = interp.call(callable, args, kwargs)
-                self.assertEqual(res, expected)
-
-        for i, ((callable, args, kwargs), expected) in enumerate([
-            ((call_func_failure, (), {}),
-             Exception),
-            ((call_func_complex, ('???',), {exc=ValueError('spam')}),
-             ValueError),
+                res = interp.call(callable)
+                self.assertIs(res, None)
+
+        for i, (callable, args, kwargs) in enumerate([
+            (call_func_ident, ('spamspamspam',), {}),
+            (get_call_func_closure, (42,), {}),
+            (get_call_func_closure(42), (), {}),
+            (Spam.from_values, (), {}),
+            (Spam.from_values, (1, 2, 3), {}),
+            (Spam, ('???'), {}),
+            (Spam(101), (), {}),
+            (Spam(10101).run, (), {}),
+            (call_func_complex, ('ident', 'spam'), {}),
+            (call_func_complex, ('full-ident', 'spam'), {}),
+            (call_func_complex, ('full-ident', 'spam', 'ham'), {'eggs': '!!!'}),
+            (call_func_complex, ('globals',), {}),
+            (call_func_complex, ('interpid',), {}),
+            (call_func_complex, ('closure',), {'value': '~~~'}),
+            (call_func_complex, ('custom', 'spam!'), {}),
+            (call_func_complex, ('custom-inner', 'eggs!'), {}),
+            (call_func_complex, ('???',), {'exc': ValueError('spam')}),
         ]):
-            with self.subTest(f'failure case #{i+1}'):
-                with self.assertRaises(expected):
-                    interp.call(callable, args, kwargs)
+            with self.subTest(f'invalid case #{i+1}'):
+                with self.assertRaises(Exception):
+                    if args or kwargs:
+                        raise Exception((args, kwargs))
+                    interp.call(callable)
+
+        with self.assertRaises(interpreters.CallFailure):
+            interp.call(call_func_failure)
 
     def test_call_in_thread(self):
         interp = interpreters.create()
@@ -850,10 +832,18 @@ def test_call_in_thread(self):
             (call_func_noop, (), {}),
             (call_func_return_shareable, (), {}),
             (call_func_return_not_shareable, (), {}),
+            (Spam.noop, (), {}),
+        ]):
+            with self.subTest(f'success case #{i+1}'):
+                with self.captured_thread_exception() as ctx:
+                    t = interp.call_in_thread(callable)
+                    t.join()
+                self.assertIsNone(ctx.caught)
+
+        for i, (callable, args, kwargs) in enumerate([
             (call_func_ident, ('spamspamspam',), {}),
             (get_call_func_closure, (42,), {}),
             (get_call_func_closure(42), (), {}),
-            (Spam.noop, (), {}),
             (Spam.from_values, (), {}),
             (Spam.from_values, (1, 2, 3), {}),
             (Spam, ('???'), {}),
@@ -867,18 +857,20 @@ def test_call_in_thread(self):
             (call_func_complex, ('closure',), {'value': '~~~'}),
             (call_func_complex, ('custom', 'spam!'), {}),
             (call_func_complex, ('custom-inner', 'eggs!'), {}),
+            (call_func_complex, ('???',), {'exc': ValueError('spam')}),
         ]):
-            with self.subTest(f'success case #{i+1}'):
-                t = interp.call_in_thread(callable, args, kwargs)
-                t.join()
-
-        for i, (callable, args, kwargs) in enumerate([
-            (call_func_failure, (), {}),
-            (call_func_complex, ('???',), {exc=ValueError('spam')}),
-        ]):
-            with self.subTest(f'failure case #{i+1}'):
-                t = interp.call_in_thread(callable, args, kwargs)
-                t.join()
+            with self.subTest(f'invalid case #{i+1}'):
                if args or kwargs:
+                    continue
+                with self.captured_thread_exception() as ctx:
+                    t = interp.call_in_thread(callable)
+                    t.join()
+                self.assertIsNotNone(ctx.caught)
+
+        with self.captured_thread_exception() as ctx:
+            t = interp.call_in_thread(call_func_failure)
+            t.join()
+        self.assertIsNotNone(ctx.caught)
 
 
 class TestIsShareable(TestBase):

@@ -4,8 +4,9 @@
 import subprocess
 import sys
 import tempfile
-import threading
 from textwrap import dedent
+import threading
+import types
 import unittest
 
 from test import support
@@ -84,6 +85,18 @@ def temp_dir(self):
         self.addCleanup(lambda: os_helper.rmtree(tempdir))
         return tempdir
 
+    @contextlib.contextmanager
+    def captured_thread_exception(self):
+        ctx = types.SimpleNamespace(caught=None)
+        def excepthook(args):
+            ctx.caught = args
+        orig_excepthook = threading.excepthook
+        threading.excepthook = excepthook
+        try:
+            yield ctx
+        finally:
+            threading.excepthook = orig_excepthook
+
     def make_script(self, filename, dirname=None, text=None):
         if text:
             text = dedent(text)

diff --git a/Modules/_xxsubinterpretersmodule.c b/Modules/_xxsubinterpretersmodule.c
@@ -902,6 +902,56 @@ The code/function must not take any arguments or be a closure\n\
 If a function is provided, its code object is used and all its state\n\
 is ignored, including its __globals__ dict.");
 
+static PyObject *
+interp_call(PyObject *self, PyObject *args, PyObject *kwds)
+{
+    static char *kwlist[] = {"id", "callable", "args", "kwargs", NULL};
+    PyObject *id, *callable;
+    PyObject *args_obj = NULL;
+    PyObject *kwargs_obj = NULL;
+    if (!PyArg_ParseTupleAndKeywords(args, kwds,
+                                     "OO|OO:" MODULE_NAME_STR ".call", kwlist,
+                                     &id, &callable, &args_obj, &kwargs_obj)) {
+        return NULL;
+    }
+
+    if (args_obj != NULL) {
+        PyErr_SetString(PyExc_ValueError, "got unexpected args");
+        return NULL;
+    }
+    if (kwargs_obj != NULL) {
+        PyErr_SetString(PyExc_ValueError, "got unexpected kwargs");
+        return NULL;
+    }
+
+    PyObject *code = (PyObject *)convert_code_arg(callable, MODULE_NAME_STR ".call",
+                                                  "argument 2", "a function");
+    if (code == NULL) {
+        return NULL;
+    }
+
+    PyObject *excinfo = NULL;
+    int res = _interp_exec(self, id, code, NULL, &excinfo);
+    Py_DECREF(code);
+    if (res < 0) {
+        assert((excinfo == NULL) != (PyErr_Occurred() == NULL));
+        return excinfo;
+    }
+    Py_RETURN_NONE;
+}
+
+PyDoc_STRVAR(call_doc,
+"call(id, callable, args=None, kwargs=None)\n\
+\n\
+Call the provided object in the identified interpreter.\n\
+Pass the given args and kwargs, if possible.\n\
+\n\
+\"callable\" may be a plain function with no free vars that takes\n\
+no arguments.\n\
+\n\
+The function's code object is used and all its state\n\
+is ignored, including its __globals__ dict.");
+
 static PyObject *
 interp_run_string(PyObject *self, PyObject *args, PyObject *kwds)
 {
@@ -1085,6 +1135,8 @@ static PyMethodDef module_functions[] = {
      METH_VARARGS | METH_KEYWORDS, is_running_doc},
     {"exec",                      _PyCFunction_CAST(interp_exec),
      METH_VARARGS | METH_KEYWORDS, exec_doc},
+    {"call",                      _PyCFunction_CAST(interp_call),
+     METH_VARARGS | METH_KEYWORDS, call_doc},
     {"run_string",                _PyCFunction_CAST(interp_run_string),
      METH_VARARGS | METH_KEYWORDS, run_string_doc},
     {"run_func",                  _PyCFunction_CAST(interp_run_func),