gh-130052: Fix some exceptions on error paths in _testexternalinspection #130053

sergey-miryanov · 2025-02-12T20:35:08Z

Some error paths don't set Exceptions.
This PR fixes them.

Issue: Some error paths do not set Exceptions in _testexternalinspection #130052

…exception

sergey-miryanov · 2025-02-13T14:16:18Z

cpython/Modules/_testexternalinspection.c

Line 917 in 67072cc

read_ptr(pid, task_address + sizeof(Py_ssize_t), &refcnt);

Refcnt doesn't used - maybe we may remove this reading?

cpython/Modules/_testexternalinspection.c

Line 1529 in 67072cc

"get_stack_trace is not supported on this platform");

This is a typo - should I fix them? get_stack_trace should be get_async_stack_trace

sergey-miryanov · 2025-02-13T14:33:21Z

@pablogsal Please take a look. I saw your fixed some calls (#129557), I added a few more exceptions. I also added a check of the result before it is used.

…exception

vstinner · 2025-02-17T13:11:24Z

@sergey-miryanov: Why did you mark this PR as a draft? Do you expect to add more changes? Or is there something else?

sergey-miryanov · 2025-02-17T13:13:07Z

@vstinner oops! I forgot to un-draft it. Thanks!

…exception

vstinner · 2025-02-17T14:37:25Z

Modules/_testexternalinspection.c

@@ -217,7 +222,9 @@ search_map_for_section(pid_t pid, const char* secname, const char* substr) {

    mach_port_t proc_ref = pid_to_task(pid);
    if (proc_ref == 0) {
-        PyErr_SetString(PyExc_PermissionError, "Cannot get task for PID");
+        if (!PyErr_Occurred()) {
+            PyErr_SetString(PyExc_PermissionError, "Cannot get task for PID");


Would it be posssible to move this code inside pid_to_task()? I'm not convinced that PermissionError is appropriate, I would suggest RuntimeError instead.

I believe it is redundant here. I will remove this check. But I saw on the internets that task_for_pid may return code == 5 (os/kern failure) that we don't check - should we?

https://github.com/attilathedud/macos_task_for_pid

https://forums.developer.apple.com/forums/thread/701647

Oh! I think PyExc_PermissionError is just exactly for this. I will move it to pid_to_task.

I kept PyExc_PermissionError and removed redundant check. Should I replace PermissionError with RuntimeError?

Modules/_testexternalinspection.c

Co-authored-by: Victor Stinner <vstinner@python.org>

sergey-miryanov · 2025-02-17T17:55:33Z

@vstinner Please take a look.

vstinner

LGTM.

miss-islington-app · 2025-02-20T17:05:43Z

Thanks @sergey-miryanov for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

miss-islington-app · 2025-02-20T17:05:46Z

Sorry, @sergey-miryanov and @vstinner, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 69426fcee7fcecbe34be66d2c5b58b6d0ffe2809 3.13

sergey-miryanov · 2025-02-20T20:44:47Z

@vstinner It looks like a lot of changes were made on 3.14 branch and not on 3.13. Should we backport this PR then?

vstinner · 2025-02-21T15:26:14Z

The automated backport failed because of merge conflicts.

@sergey-miryanov: Can you please try to backport this change manually to 3.13?

sergey-miryanov · 2025-02-21T16:40:01Z

@vstinner I tried yesterday yet. There are commits that not backported:

* 69426fcee7f gh-130052: Fix some exceptions on error paths in _testexternalinspection (#130053)
* 49b11033bd8 GH-91048: Correct error path in testexternalinspection (#129557)
* 633853004c5 gh-130050: Fix memory leaks in _testexternalinspection (#130051)
* 7eaef74561c gh-129430: Make walking vm regions more efficient in MacOS (#129494)
* be98fda7c66 gh-129223: Raise KeyError in search_map_for_section() if not found (#129262)
* 3a3a6b86f40 gh-129223: Do not allow the compiler to optimise away symbols for debug sections (#129225)
* 67d804b494d GH-91048: Don't attempt to run on FreeBSD (#129189)
* 188598851d5 GH-91048: Add utils for capturing async call stack for asyncio programs and enable profiling (#124640)
* f5b6356a11b GH-128563: Add new frame owner type for interpreter entry frames (GH-129078)
* 6d93690954d gh-125604: Move _Py_AuditHookEntry, etc. Out of pycore_runtime.h (gh-125605)
* b2afe2aae48 gh-123923: Defer refcounting for `f_executable` in `_PyInterpreterFrame` (#123924)

Should I backport them all?

vstinner · 2025-02-21T16:49:24Z

@pablogsal: Modules/_testexternalinspection.c lacks many backports in the 3.13 branch. Is it a deliberate choice? Or should we backport these changes to 3.13?

pablogsal · 2025-02-21T16:54:37Z

@pablogsal: Modules/_testexternalinspection.c lacks many backports in the 3.13 branch. Is it a deliberate choice? Or should we backport these changes to 3.13?

3.13 has half the test cases because there is no async-related introspection and most of the bugs we have been fixing are in the new code. I will try to go over the PRs and see if anything applies to the other code that we had

Set exception in read_memory if not supported on the platform

23fdc5f

bedevere-app bot added tests Tests in the Lib/test dir awaiting review labels Feb 12, 2025

bedevere-app bot mentioned this pull request Feb 12, 2025

Some error paths do not set Exceptions in _testexternalinspection #130052

Closed

sergey-miryanov marked this pull request as draft February 12, 2025 20:35

bedevere-app bot removed the awaiting review label Feb 12, 2025

sergey-miryanov mentioned this pull request Feb 12, 2025

Python 3.14.0a5: test_external_inspection fails on freethreading-debug build on s390x Linux #130035

Closed

encukou added the skip news label Feb 13, 2025

sergey-miryanov added 3 commits February 13, 2025 18:55

Merge branch 'main' into pythongh-130052-test-external-inspection-set…

9679c08

…exception

Add more exceptions to _testexternalinspection

5e15349

Fix PyErr_Format call

67072cc

sergey-miryanov added 2 commits February 13, 2025 19:33

Fix small typo

b8c800c

Merge branch 'main' into pythongh-130052-test-external-inspection-set…

4830892

…exception

sergey-miryanov marked this pull request as ready for review February 17, 2025 13:13

bedevere-app bot added the awaiting review label Feb 17, 2025

Merge branch 'main' into pythongh-130052-test-external-inspection-set…

8bf3c22

…exception

vstinner reviewed Feb 17, 2025

View reviewed changes

sergey-miryanov and others added 3 commits February 17, 2025 22:02

Update Modules/_testexternalinspection.c

6dfc0f0

Co-authored-by: Victor Stinner <vstinner@python.org>

Remove redundant check of pid_to_task result

e9ac75b

Fit lines to 80 columns

4fde8de

vstinner approved these changes Feb 20, 2025

View reviewed changes

bedevere-app bot added awaiting merge and removed awaiting review labels Feb 20, 2025

vstinner added the needs backport to 3.13 bugs and security fixes label Feb 20, 2025

vstinner enabled auto-merge (squash) February 20, 2025 17:05

vstinner merged commit 69426fc into python:main Feb 20, 2025
47 checks passed

bedevere-app bot removed the awaiting merge label Feb 20, 2025

miss-islington-app bot assigned vstinner Feb 20, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

gh-130052: Fix some exceptions on error paths in _testexternalinspection #130053

gh-130052: Fix some exceptions on error paths in _testexternalinspection #130053

sergey-miryanov commented Feb 12, 2025 •

edited by bedevere-app bot

Loading

sergey-miryanov commented Feb 13, 2025 •

edited

Loading

sergey-miryanov commented Feb 13, 2025

vstinner commented Feb 17, 2025

sergey-miryanov commented Feb 17, 2025 •

edited

Loading

vstinner Feb 17, 2025

sergey-miryanov Feb 17, 2025

sergey-miryanov Feb 17, 2025

sergey-miryanov Feb 17, 2025

sergey-miryanov commented Feb 17, 2025

vstinner left a comment

miss-islington-app bot commented Feb 20, 2025

miss-islington-app bot commented Feb 20, 2025

sergey-miryanov commented Feb 20, 2025

vstinner commented Feb 21, 2025

sergey-miryanov commented Feb 21, 2025 •

edited

Loading

vstinner commented Feb 21, 2025

pablogsal commented Feb 21, 2025

gh-130052: Fix some exceptions on error paths in _testexternalinspection #130053

gh-130052: Fix some exceptions on error paths in _testexternalinspection #130053

Conversation

sergey-miryanov commented Feb 12, 2025 • edited by bedevere-app bot Loading

sergey-miryanov commented Feb 13, 2025 • edited Loading

sergey-miryanov commented Feb 13, 2025

vstinner commented Feb 17, 2025

sergey-miryanov commented Feb 17, 2025 • edited Loading

vstinner Feb 17, 2025

Choose a reason for hiding this comment

sergey-miryanov Feb 17, 2025

Choose a reason for hiding this comment

sergey-miryanov Feb 17, 2025

Choose a reason for hiding this comment

sergey-miryanov Feb 17, 2025

Choose a reason for hiding this comment

sergey-miryanov commented Feb 17, 2025

vstinner left a comment

Choose a reason for hiding this comment

miss-islington-app bot commented Feb 20, 2025

miss-islington-app bot commented Feb 20, 2025

sergey-miryanov commented Feb 20, 2025

vstinner commented Feb 21, 2025

sergey-miryanov commented Feb 21, 2025 • edited Loading

vstinner commented Feb 21, 2025

pablogsal commented Feb 21, 2025

sergey-miryanov commented Feb 12, 2025 •

edited by bedevere-app bot

Loading

sergey-miryanov commented Feb 13, 2025 •

edited

Loading

sergey-miryanov commented Feb 17, 2025 •

edited

Loading

sergey-miryanov commented Feb 21, 2025 •

edited

Loading