Repository navigation
Check usage of Py_EnterRecursiveCall() and Py_LeaveRecursiveCall() in new FASTCALL functions #73492
Description
| BPO | 29306 |
|---|---|
| Nosy | @vstinner, @methane |
| Files |
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 2018-09-19.23:30:23.704>
created_at = <Date 2017-01-18.09:04:44.836>
labels = ['interpreter-core', 'type-bug', '3.7']
title = 'Check usage of Py_EnterRecursiveCall() and Py_LeaveRecursiveCall() in new FASTCALL functions'
updated_at = <Date 2018-09-19.23:30:23.703>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2018-09-19.23:30:23.703>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2018-09-19.23:30:23.704>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2017-01-18.09:04:44.836>
creator = 'vstinner'
dependencies = []
files = ['46327', '46605', '46607']
hgrepos = []
issue_num = 29306
keywords = ['patch']
message_count = 13.0
messages = ['285709', '285713', '285727', '287300', '287303', '287305', '287307', '287308', '287314', '287404', '287411', '287412', '325828']
nosy_count = 3.0
nosy_names = ['vstinner', 'methane', 'python-dev']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue29306'
versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']
Activity
vstinner commentedon Jan 18, 2017
I added the following new functions to Python 3.6:
I'm not sure that these functions update correctly the "recursion_depth" counter using Py_EnterRecursiveCall() and Py_LeaveRecursiveCall().
methane commentedon Jan 18, 2017
I think basic idea is "wrap unknown function call at least once, and preferably only once".
_PyEval_EvalFrameDefault() calls Py_EnterRecursiveCall("").
So wrapping PyFunction_(Fast)Call* with Py_EnterRecursiveCall() seems redundant.
On the other hand, PyCFunction may calls method of extension module,
and it can cause recursive call.
So I think PyCFunction_Call methods calling function pointer it has
(e.g. _PyCFunction_FastCallKeywords) should call Py_EnterRecursiveCall().
And caller of them can skip Py_EnterRecursiveCall().
For example, _PyObject_FastCallDict() may call _PyFunction_FastCallDict() or
_PyCFunction_FastCallDict(), or tp_fastcall (via _Py_RawFastCallDict) or tp_call.
It can wrap all, but wrapping only tp_fastcall and tp_call is better.
vstinner commentedon Jan 18, 2017
Naoki: "On the other hand, PyCFunction may calls method of extension module, and it can cause recursive call. So I think PyCFunction_Call methods calling function pointer it has (e.g. _PyCFunction_FastCallKeywords) should call Py_EnterRecursiveCall()."
In Python 3.5, C functions are only calls inside Py_EnterRecursiveCall() if calls from PyObject_Call(). There are many other ways to call C functions which don't use Py_EnterRecursiveCall(). Examples:
call_function() and do_call() have a special case for PyCFunction, otherwise they call the generic PyObject_Call() which uses Py_EnterRecursiveCall().
I agree that calling C functions with Py_EnterRecursiveCall() would help to prevent stack overflow crashs.
Attached enter_recursive_call.patch patch:
python-dev commentedon Feb 8, 2017
New changeset 88ed9d9eabc1 by Victor Stinner in branch 'default':
Issue bpo-29306: Fix usage of Py_EnterRecursiveCall()
https://hg.python.org/cpython/rev/88ed9d9eabc1
vstinner commentedon Feb 8, 2017
I still need to backport fixes to Python 3.6, maybe even Python 3.5.
python-dev commentedon Feb 8, 2017
New changeset 65d24ff4bbd3320acadb58a5e4d944c84536cb2c by Victor Stinner in branch 'master':
Issue bpo-29306: Fix usage of Py_EnterRecursiveCall()
65d24ff
vstinner commentedon Feb 8, 2017
I needed this fix to work on issue bpo-29465. I expected that my patch was reviewed, but woops, it wasn't the case and I missed a refleak. Hopefully, the refleak is now fixed!
python-dev commentedon Feb 8, 2017
New changeset 37705f89c72b by Victor Stinner in branch 'default':
Fix refleaks if Py_EnterRecursiveCall() fails
https://hg.python.org/cpython/rev/37705f89c72b
python-dev commentedon Feb 8, 2017
New changeset 1101819ba99afcb4d1b6495d49b17bdd0acfe761 by Victor Stinner in branch 'master':
Fix refleaks if Py_EnterRecursiveCall() fails
1101819
vstinner commentedon Feb 9, 2017
enter_recursive_call_36.patch: Patch for Python 3.6.
vstinner commentedon Feb 9, 2017
check_recursion_depth.py: script using Python functions and C function (functools.partial) to check the recursion depth.
Example with Python 3.6:
haypo@selma$ ./python check_recursion_depth.py # unpatched
got: 148, expected: 100
haypo@selma$ ./python check_recursion_depth.py # patched
got: 100, expected: 100
Maybe we need "proxies" in _testcapi for each kind of function, a function taking a callback and calling this function. Function types:
vstinner commentedon Feb 9, 2017
I tested check_recursion_depth.py: Python 2.7, 3.5 and 3.6 have the bug. Python 3.7 is already fixed.
vstinner commentedon Sep 19, 2018
I'm not sure about touching the stable branches. At least, the issue has been fixed since Python 3.7. I close the issue.