gh-132775: Expand the Capability of Interpreter.call() #133484

ericsnowcurrently · 2025-05-06T00:15:48Z

It now supports most callables, full args, and return values.

Issue: Improvements to test.support.interpreters.Interpreter #132775

neonene · 2025-05-13T07:01:24Z

Lib/test/support/interpreters/__init__.py

+    def call(self, callable, /, *args, **kwargs):
        """Call the object in the interpreter with given args/kwargs.

        Only functions that take no arguments and have no closure


The docstring looks partially outdated (here or in Lib/interpreters/__init__.py). ~~The same would apply for _interpretersmodule.c?~~

neonene

This review focuses on the refleaks in test_api (Py_GIL_DISABLED seems to skip the test).

_PyFunction_VerifyStateless:

cpython/Objects/funcobject.c

Line 1267 in 0d499c7

if (defaults != NULL && defaults != Py_None && PyDict_Size(defaults) > 0)

PyDict_Size(defaults) needs PyDict_Check() or _PyErr_Clear().
The same goes for PyDict_Size(kwdefaults) at L1275.

neonene · 2025-05-28T14:19:21Z

Modules/_interpretersmodule.c

+    struct interp_call temp = *call;
+    *call = (struct interp_call){0};
+    if (temp.func != NULL) {
+        _PyXIData_Clear(NULL, temp.func);


_PyXIData_Clear() here does not work expectedly on MSVC.

struct interp_call temp = *call; printf("before: %p %p %p\n", temp.func, temp.func->data, temp.func->obj); *call = (struct interp_call){0}; printf("after : %p %p %p\n", temp.func, temp.func->data, temp.func->obj); before: 0000000000C3CF58 00000000057763B0 0000000002BFCEF0 after : 0000000000C3CF58 0000000000000000 0000000000000000

Yeah, the logic here is wrong because the pointers in temp point to the call fields, which have been nulled out. I'm fixing that.

neonene · 2025-05-28T14:26:34Z

Modules/_interpretersmodule.c

+        PyObject *exc = _PyErr_GetRaisedException(tstate);
+        if (_PyPickle_GetXIData(tstate, func, &call->_preallocated.func) < 0) {
+            _PyErr_SetRaisedException(tstate, exc);
+//unwrap_not_shareable(tstate);
+            return -1;
+        }


Py_DECREF(exc) seems missing after L472.

neonene · 2025-05-28T14:32:03Z

Modules/_interpretersmodule.c

    }
-
-    _PyXI_FreeSession(session);
    return res;


Py_CLEAR(result.preserved) before L665 seems effective?

neonene · 2025-05-28T14:34:06Z

Modules/_interpretersmodule.c

+    PyObject *func, *args, *kwargs;
+    if (_interp_call_unpack(call, &func, &args, &kwargs) == 0) {
+        // Either the problem is intermittent or only affects subinterpreters.
+        // This is highly unlikely.


Py_DECREF(func); Py_DECREF(args); Py_XDECREF(kwargs);?

neonene · 2025-05-28T14:39:27Z

Modules/_interpretersmodule.c

-    if (kwargs_obj != NULL) {
-        _PyErr_SetString(tstate, PyExc_ValueError, "got unexpected kwargs");
+    struct interp_call call = {0};
+    if (_interp_call_pack(tstate, &call, callable, args_obj, kwargs_obj) < 0) {


_interp_call_clear(&call) in the branch?

ericsnowcurrently · 2025-05-28T16:56:31Z

PyDict_Size(defaults) needs PyDict_Check() or _PyErr_Clear().

The same goes for PyDict_Size(kwdefaults) at L1275.

Nice catch. I also got it wrong with defaults because it's supposed to be a tuple, not a dict.

ericsnowcurrently · 2025-05-29T18:46:07Z

!buildbot AMD64 Fedora Stable Refleaks

bedevere-bot · 2025-05-29T18:46:10Z

🤖 New build scheduled with the buildbot fleet by @ericsnowcurrently for commit 17beeda 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F133484%2Fmerge

The command will test the builders whose names match following regular expression: AMD64 Fedora Stable Refleaks

The builders matched are:

AMD64 Fedora Stable Refleaks PR

ericsnowcurrently · 2025-05-29T20:10:40Z

@neonene, thanks again for all the help chasing down leaks! You saved me a bunch of time.

ericsnowcurrently · 2025-05-29T21:04:14Z

Unless there are any objections, I'll merge this first thing in the morning (~16:00 UTC).

miss-islington-app · 2025-05-30T15:15:03Z

Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

…h-133484) It now supports most callables, full args, and return values. (cherry picked from commit 52deabe) Co-authored-by: Eric Snow <[email protected]>

bedevere-app · 2025-05-30T15:15:16Z

GH-134933 is a backport of this pull request to the 3.14 branch.

) It now supports most callables, full args, and return values. (cherry picked from commit 52deabe, AKA gh-133484) Co-authored-by: Eric Snow [email protected]

ericsnowcurrently added the skip news label May 6, 2025

bedevere-app bot mentioned this pull request May 6, 2025

Improvements to test.support.interpreters.Interpreter #132775

Closed

ericsnowcurrently force-pushed the interp-call-expansion branch from 4fdc11a to 4327098 Compare May 6, 2025 17:36

ericsnowcurrently added the needs backport to 3.14 bugs and security fixes label May 8, 2025

ericsnowcurrently force-pushed the interp-call-expansion branch 3 times, most recently from 27d83f1 to f9f7666 Compare May 13, 2025 01:04

ericsnowcurrently mentioned this pull request May 13, 2025

gh-133485: Use _interpreters.call() in InterpreterPoolExecutor #133957

Open

ericsnowcurrently force-pushed the interp-call-expansion branch from f9f7666 to b8171d9 Compare May 13, 2025 01:13

neonene reviewed May 13, 2025

View reviewed changes

ericsnowcurrently force-pushed the interp-call-expansion branch 4 times, most recently from d58f54e to 4152f17 Compare May 22, 2025 15:16

ericsnowcurrently added 4 commits May 22, 2025 10:32

Add _PyXI_session_result.

cc387f8

Add _PyXI_Preserve().

b906d4a

Support full calls in interp_call().

08307a0

Use _PyXI_Preserve().

d173ec0

ericsnowcurrently force-pushed the interp-call-expansion branch from 4152f17 to d173ec0 Compare May 22, 2025 16:33

ericsnowcurrently marked this pull request as ready for review May 22, 2025 16:34

bedevere-app bot added the awaiting core review label May 22, 2025

ericsnowcurrently added 4 commits May 26, 2025 11:32

Update the Interpreter.call() docstring.

a9612e3

Improve handling of xidata-to-object for scripts and calls.

5d7141d

Merge branch 'main' into interp-call-expansion

293a4dc

Always load the missing module in XI data situations.

c25d9e1

ericsnowcurrently requested a review from kumaraditya303 as a code owner May 26, 2025 22:55

ericsnowcurrently removed the request for review from kumaraditya303 May 26, 2025 22:59

This comment was marked as outdated.

Sign in to view

neonene reviewed May 28, 2025

View reviewed changes

ericsnowcurrently added 3 commits May 28, 2025 12:09

Fix refleaks.

66bed1d

Fix _PyFunction_VerifyStateless().

3ed3423

Fix exc propagation and an excinfo memory leak.

17beeda

Drop a stale comment.

2682157

Merge branch 'main' into interp-call-expansion

37c41f8

ericsnowcurrently merged commit 52deabe into python:main May 30, 2025
38 checks passed

bedevere-app bot removed the awaiting core review label May 30, 2025

ericsnowcurrently deleted the interp-call-expansion branch May 30, 2025 15:15

bedevere-app bot removed the needs backport to 3.14 bugs and security fixes label May 30, 2025

Uh oh!

gh-132775: Expand the Capability of Interpreter.call() #133484

gh-132775: Expand the Capability of Interpreter.call() #133484

Conversation

ericsnowcurrently commented May 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

neonene May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

neonene left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ericsnowcurrently commented May 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ericsnowcurrently commented May 29, 2025

Uh oh!

bedevere-bot commented May 29, 2025

Uh oh!

ericsnowcurrently commented May 29, 2025

Uh oh!

ericsnowcurrently commented May 29, 2025

Uh oh!

Uh oh!

miss-islington-app bot commented May 30, 2025

Uh oh!

bedevere-app bot commented May 30, 2025

Uh oh!

Uh oh!

ericsnowcurrently commented May 6, 2025 •

edited

Loading

neonene May 13, 2025 •

edited

Loading

neonene left a comment •

edited

Loading

ericsnowcurrently commented May 28, 2025 •

edited

Loading