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

[sergey-miryanov]

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

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

[sergey-miryanov]

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]

@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.

[vstinner]

@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]

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

[vstinner]

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.

[sergey-miryanov]

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

[sergey-miryanov]

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

[sergey-miryanov]

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

[sergey-miryanov]

@vstinner Please take a look.

[vstinner]

LGTM.

[miss-islington-app]

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]

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]

@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]

The automated backport failed because of merge conflicts.

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

[sergey-miryanov]

@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]

@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]

@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

[serhiy-storchaka]

Reminder about backporting. @sergey-miryanov

[sergey-miryanov]

I'm afraid it is on @pablogsal side now. I'm not against of backporting those commits and if we decide to do this I'm ready to work on backporting.

[serhiy-storchaka]

If it was decided to not backport this, please remove the "needs backport to 3.13" label.