Recommend PyObject_Call* APIs over PyEval_Call*() APIs

[methane]

BPO	29548
Nosy	@malemburg, @rhettinger, @mdickinson, @ncoghlan, @vstinner, @methane, @serhiy-storchaka, @jdemeyer
PRs	bpo-29548: Recommend PyObject_Call APIs over PyEval_Call APIs. #75 bpo-29684: Fix regression of PyEval_CallObjectWithKeywords #87 bpo-29548: Fix some inefficient call API usage #97 bpo-29548: no longer use PyEval_Call* functions #14683 bpo-29548: deprecate PyEval_Call* functions #14804

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2017-03-14.09:01:29.804>
created_at = <Date 2017-02-13.17:36:52.181>
labels = ['interpreter-core', '3.7']
title = 'Recommend PyObject_Call* APIs over PyEval_Call*() APIs'
updated_at = <Date 2019-07-24.12:03:11.163>
user = 'https://github.com/methane'

bugs.python.org fields:

activity = <Date 2019-07-24.12:03:11.163>
actor = 'methane'
assignee = 'none'
closed = True
closed_date = <Date 2017-03-14.09:01:29.804>
closer = 'methane'
components = ['Interpreter Core']
creation = <Date 2017-02-13.17:36:52.181>
creator = 'methane'
dependencies = []
files = []
hgrepos = []
issue_num = 29548
keywords = []
message_count = 29.0
messages = ['287714', '287717', '287732', '287734', '287742', '287743', '287744', '287752', '287758', '287759', '287760', '287763', '287766', '287767', '287768', '287769', '287775', '287777', '287829', '287833', '287834', '287837', '287865', '287907', '290186', '347664', '347689', '347690', '348382']
nosy_count = 8.0
nosy_names = ['lemburg', 'rhettinger', 'mark.dickinson', 'ncoghlan', 'vstinner', 'methane', 'serhiy.storchaka', 'jdemeyer']
pr_nums = ['75', '87', '97', '14683', '14804']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue29548'
versions = ['Python 3.7']

[methane]

As reading call.c, PyEval_Call* APIs are mostly duplicate of PyObject_Call* APIs.

While they are not documented, they seems part of limited APIs.
So we can't remove them easily. Let's deprecate them for now.

[malemburg]

Please note that the two sets of APIs are not identical, e.g. you cannot simply replace PyEval_CallObject() with PyObject_Call(), since the former applies a few extra checks and defaults, which the latter doesn't.

[methane]

Thanks, Lemburg to pointing it out.
Here is detail of the difference.

## PyEval_CallFunction(), PyEval_CallMethod()

They are very similar to PyObject_CallFunction() and PyObject_CallMethod(). difference are:

• PyEval_Call...() doesn't respect Py_SSIZE_T_CLEAN
• PyObject_Call... has following special case. PyEval_CallFunction(callable, "i", (int)i) will raise TypeError("keyword list must be a tuple") and PyObject_CallFunction(callable, "i", (int)i) calls callable(i)

 if (nargs == 1 && PyTuple_Check(stack[0])){
 /* Special cases for backward compatibility:
 - PyObject_CallFunction(func, "O", tuple) calls func(*tuple)
 - PyObject_CallFunction(func, "(OOO)", arg1, arg2, arg3) calls
 func(*(arg1, arg2, arg3)): func(arg1, arg2, arg3) */
 PyObject *args = stack[0];
 result = _PyObject_FastCall(callable,
 &PyTuple_GET_ITEM(args, 0),
 PyTuple_GET_SIZE(args));
 }

PyEval_CallFunction is not called from Python source tree.
PyEval_CallMethod has only one caller in tree and format string is "(Oi)". It can be replaced with PyObject_CallMethod safely.

## PyEval_CallObject(), PyEval_CallObjectWithKeywords()

PyEval_CallObject() is a just macro calling PyEval_CallObjectWithKeywords() (call it COWK later).
PyEval_CallObject() is identical to PyObject_CallObject(). Only difference is it's a macro or function.

COWK is similar to PyObject_Call(), but COWK raise TypeError when args is not a tuple or kwds is not a dictionary and PyObject_Call() uses assert.

There are only two caller of PyEval_CallObjectWithKeywords() other than PyEval_CallObject and PyObject_CallObject.
One is tp_call of weakcallableproxy. Type of kwargs is checked before calling tp_call.
Another is in threading (boot->keyw). The threading module checks it's a dict.
So replacing them to PyObject_CallObject() is safe.

----

While they are complex API, there are few (or no) callers in Python tree.
It's too hard to maintain. Actually, I found regression of COWK in Python 3.6.

155ea65

It calls _PyObject_FastCallDict() when args is NULL. If kwargs is not dict, it can crash instead of raising TypeError.

[methane]

PR 87 fixes the regression in 3.6 branch

[mdickinson]

See also bpo-11165

[serhiy-storchaka]

See also bpo-485165, bpo-8276 and bpo-11173.

[malemburg]

Thanks, but you missed the main difference:

The argument tuple can be NULL for PyEval_CallObject() (it then gets replaced with an empty tuple), but will raise a segfault for PyObject_Call().

Now, apart from looking at the use cases in the core, you also have to check whether you are removing useful functionality when deprecating APIs. Code in third party extensions may not be that easy to adapt to the PyObject_Call*() APIs, since they lack several checks which the PyEval_Call*() APIs apply.

Deprecation of existing published APIs is only an option in case there are other APIs which can reasonably replace them and those other APIs would have to have been around for a while, since otherwise third party code would have to provide wrappers for Python versions which don't supply these APIs or only ones which do not implement the extra functionality.

E.g. if you now add the extra support for args being NULL to PyObject_Call() in Python 3.7, third party code would have to either switch being using PyEval_CallObject() and PyObject_Call() depending on Python version or provide its own wrappers around the two APIs depending on Python version.

Since calling objects is rather common in Python extensions, special care has to be taken.

It would have been better to add the special case for args == NULL and the extra types checks to PyObject_Call() a long long time ago, since it's the only API that allows calling an object with both args and kwargs. Alas, didn't happen, so we have to live with it.

It may actually be better to add a new API PyObject_CallObjectWithKeywords() which works like the PyEval_CallObjectWithKeywords() API and then deprecate the PyEval_COWK() API. Third party code can then use a simple macro to provide a backwards compatibility shim for older Python versions.

[methane]

The argument tuple can be NULL for PyEval_CallObject() (it then gets replaced with an empty tuple), but will raise a segfault for PyObject_Call().

PyObject_CallObject() accepts NULL as args. Macro vs function is only difference of them.

On the other hand, PyObject_CallObjectWithKeyword() doesn't have identical function.
So I agree your suggestion to add NULL support to PyObject_Call().

We have more fast _PyObject_FastCall* APIs for performance critical code.
Additional cost to check NULL in PyObject_Call() would be negligible.

Any opinion from others?

[malemburg]

Looking through Python's history, it's interesting that PyObject_Call() did apply the args == NULL checks up until Python 2.1.

In Python 2.2 this was replaced by a direct call to tp_call, without the checks. However, the tp_call slots don't do this check as you can see in function_call() of function objects. And indeed, the documentation of PyObject_Call() was changed in that version as well to disallow args == NULL.

[vstinner]

"you now add the extra support for args being NULL to PyObject_Call()"

I dislike this idea. I don't want to change the API of this function.

If it is likely that NULL is the result of a previous error:

args = Py_BuildValue(...);
res = PyObject_Call(func, args, NULL);

There are already enough ways to call a function with no argument:

res = PyObject_CallFunction(func, NULL);
res = PyObject_CallFunctionObjArgs(func, NULL);
res = _PyObject_CallNoArg(func) # private

If you want to call a function only with keyword arguments, well, create an empty tuple ... which is a singleton in CPython, no risk of memory allocation failure ... and use PyObject_Call(), no?

[vstinner]

Please note that the two sets of APIs are not identical, e.g. you cannot simply replace PyEval_CallObject() with PyObject_Call(), since the former applies a few extra checks and defaults, which the latter doesn't.

IMHO these checks are too expensive at runtime for little benefit. If you pass non-tuple to PyObject_Call(), Python immediately crash. You are immediately noticied of the bug :-) I don't think that such bugs are common enough to justify the overhead.

Any idea of the popularity of the undocumented PyEval_xxx() functions? Are they used by Cython for example? By a single random extension module in the world?

I'm more in favor of modifying PyEval_xxx() to call PyObject_xxx() and deprecate them.

[methane]

I'm more in favor of modifying PyEval_xxx() to call PyObject_xxx() and deprecate them.

That's PR 75 :)

[malemburg]

On 14.02.2017 11:16, STINNER Victor wrote:

> Please note that the two sets of APIs are not identical, e.g. you cannot simply replace PyEval_CallObject() with PyObject_Call(), since the former applies a few extra checks and defaults, which the latter doesn't.

IMHO these checks are too expensive at runtime for little benefit. If you pass non-tuple to PyObject_Call(), Python immediately crash. You are immediately noticied of the bug :-) I don't think that such bugs are common enough to justify the overhead.

From the design of the abstract API layer, it is rather
uncommon to have these not do extra checks to prevent segfaults.
They were originally designed to be safe and developer friendly.

OTOH, the PyEval_* APIs were designed to be fast, only for people
who know what they are doing and for interpreter internals.

Historically, this design approach appears to have been swapped
somewhere between Python 2.1 and 2.2 for the call APIs,
which is unfortunate.

So from a design perspective, it would be better to have the
abstract APIs again do proper checks and leave the low level,
"segfault protected" :-) call APIs around as PyEval_Call*()
or better yet: not make them public at all.

Any idea of the popularity of the undocumented PyEval_xxx() functions? Are they used by Cython for example? By a single random extension module in the world?

Well, I know that our eGenix extensions are using them
and there are quite a few hits on github as well:

https://github.com/search?utf8=%E2%9C%93&q=PyEval_CallObjectWithKeywords&type=Code&ref=searchresults

I'm more in favor of modifying PyEval_xxx() to call PyObject_xxx() and deprecate them.