Bug 229824 - [webkitpy] WrappedPopen breaks process returncode
Summary: [webkitpy] WrappedPopen breaks process returncode
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-09-02 12:42 PDT by Carlos Alberto Lopez Perez
Modified: 2021-09-02 14:10 PDT (History)
4 users (show)

See Also:


Attachments
Patch (1.67 KB, patch)
2021-09-02 12:55 PDT, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Alberto Lopez Perez 2021-09-02 12:42:43 PDT
The class WrappedPopen() was added in r250375 to wrap the subprocess.Popen() for making the python2 subprocess.Popen() objects more similar to the ones from python3.

But the way this class wraps the objects makes breaks the process returncode if the process doesn't return immediately.

This is because WrappedPopen() defines a new class the sets functions with the same name than subprocess.Popen() setting the value returned to the one that returns subprocess.Popen().
But in the case of subprocess.Popen().returncode that is not a function, but a string that will change of value when the process ends. If we evaluate this function before we call subprocess.Popen().communicate() or subprocess.Popen().wait() then the value of returncode will be None

This can be checked with this following test program: http://sprunge.us/aoxZ0j

If you run it with python2 you get "The call return code is None" instead of getting "The call return code is 2" which is what it should be (what happens when you run it with python3 which don't triggers the call to use WrappedPopen())

I have observed this when working on bug 229758 .. with python2 the call from host.executive.popen() always returned a returncode of None

This is not an issue for subprocess.Popen().stdout and subprocess.Popen().stderr because if you look at what prints the debug function on that example for this cases it sets the value to a descriptor rather than reading from the descriptor. So when the call to read from the descriptor is done it will work. Example, you see this:

setting attribute .. stdout to <open file '<fdopen>', mode 'rb' at 0x7fe12cc1d540>
setting attribute .. returncode to None
Comment 1 Jonathan Bedard 2021-09-02 12:55:48 PDT
Created attachment 437186 [details]
Patch
Comment 2 Jonathan Bedard 2021-09-02 12:56:37 PDT
(In reply to Jonathan Bedard from comment #1)
> Created attachment 437186 [details]
> Patch

I suspect this will fix the problem, although I'm a bit unclear which script can cause this.
Comment 3 Carlos Alberto Lopez Perez 2021-09-02 13:25:20 PDT
This fixes it indeed, thanks.

I was trying here to instead of wrapping the object to add the __exit__ and __enter__ methods to the Popen class directly, but for some unknown reason then when you try to use the usual "with popen_object:" it doesn't like this methods added via setattr() and complains about missing __exit__
Comment 4 EWS 2021-09-02 14:09:22 PDT
Committed r281952 (241259@main): <https://commits.webkit.org/241259@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 437186 [details].
Comment 5 Radar WebKit Bug Importer 2021-09-02 14:10:31 PDT
<rdar://problem/82692759>