Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gh-98257: Make _PyEval_SetTrace() reentrant #98258

Merged
merged 2 commits into from
Oct 19, 2022
Merged

gh-98257: Make _PyEval_SetTrace() reentrant #98258

merged 2 commits into from
Oct 19, 2022

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Oct 14, 2022

Make sys.setprofile() and sys.settrace() functions reentrant. They can no long fail with: RuntimeError("Cannot install a trace function while another trace function is being installed").

Make _PyEval_SetTrace() and _PyEval_SetProfile() functions reentrant, rather than detecting and rejecting reentrant calls. Only delete the reference to function arguments once the new function is fully set, when a reentrant call is safe. Call also _PySys_Audit() earlier.

Make sys.setprofile() and sys.settrace() functions reentrant.  They
can no long fail with: RuntimeError("Cannot install a trace function
while another trace function is being installed").

Make _PyEval_SetTrace() and _PyEval_SetProfile() functions reentrant,
rather than detecting and rejecting reentrant calls. Only delete the
reference to function arguments once the new function is fully set,
when a reentrant call is safe. Call also _PySys_Audit() earlier.
@vstinner
Copy link
Member Author

@markshannon @pablogsal: Would you mind to review this change?

The reentrant call introduced by the commit 40d81fd to fix gh-94510:

commit 40d81fd63b46cf998880ce3bf3e5cb42bc3199c1
Author: Pablo Galindo Salgado <Pablogsal@gmail.com>
Date:   Tue Jul 5 19:18:47 2022 +0100

    gh-94510: Raise on re-entrant calls to sys.setprofile and sys.settrace (GH-94511)
    
    Co-authored-by: Łukasz Langa <lukasz@langa.pl>

@vstinner
Copy link
Member Author

vstinner commented Oct 14, 2022

This change fix #98257 on my Linux machine (without this fix, it takes less than a minute to reproduce it). I interrupted the test after 10 min:

$ ./python -m test test_import -m test_concurrency -j50 -F --fail-env-changed  
(...)
0:09:58 load avg: 51.58 [4904] test_import passed
0:09:58 load avg: 51.58 [4905] test_import passed
0:09:58 load avg: 51.58 [4906] test_import passed
0:09:58 load avg: 51.58 [4907] test_import passed
0:09:58 load avg: 51.58 [4908] test_import passed
0:09:58 load avg: 51.58 [4909] test_import passed
0:09:58 load avg: 51.58 [4910] test_import passed
0:09:58 load avg: 51.58 [4911] test_import passed
^C

I don't see any refleak regression:

$ ./python -m test test_sys_setprofile test_sys_settrace test_import -j3 -R 3:3
(...)
All 3 tests OK.

@ericsnowcurrently
Copy link
Member

CC @brandtbucher

@vstinner vstinner merged commit a8fe4bb into python:main Oct 19, 2022
@vstinner vstinner deleted the reentrant_settrace branch October 19, 2022 22:31
carljm added a commit to carljm/cpython that referenced this pull request Oct 20, 2022
* main: (40 commits)
  pythongh-98461: Fix source location in comprehensions bytecode (pythonGH-98464)
  pythongh-98421: Clean Up PyObject_Print (pythonGH-98422)
  pythongh-98360: multiprocessing now spawns children on Windows with correct argv[0] in virtual environments (pythonGH-98462)
  CODEOWNERS: Become a typing code owner (python#98480)
  [doc] Improve logging cookbook example. (pythonGH-98481)
  Add more tkinter.Canvas tests (pythonGH-98475)
  pythongh-95023: Added os.setns and os.unshare functions (python#95046)
  pythonGH-98363: Presize the list for batched() (pythonGH-98419)
  pythongh-98374: Suppress ImportError for invalid query for help() command. (pythongh-98450)
  typing tests: `_overload_dummy` raises `NotImplementedError`, not `RuntimeError` (python#98351)
  pythongh-98354: Add unicode check for 'name' attribute in _imp_create_builtin (pythonGH-98412)
  pythongh-98257: Make _PyEval_SetTrace() reentrant (python#98258)
  pythongh-98414: py.exe launcher does not use defaults for -V:company/ option (pythonGH-98460)
  pythongh-98417: Store int_max_str_digits on the Interpreter State (pythonGH-98418)
  Doc: Remove title text from internal links (python#98409)
  [doc] Refresh the venv introduction documentation, and correct the statement about VIRTUAL_ENV (pythonGH-98350)
  Docs: Bump sphinx-lint and fix unbalanced inline literal markup (python#98441)
  pythongh-92886: Replace assertion statements in `handlers.BaseHandler` to support running with optimizations (`-O`) (pythonGH-93231)
  pythongh-92886: Fix tests that fail when running with optimizations (`-O`) in `_test_multiprocessing.py` (pythonGH-93233)
  pythongh-92886: Fix tests that fail when running with optimizations (`-O`) in `test_py_compile.py` (pythonGH-93235)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants