[PATCH] python: fix Popen.wait() deadlock issue

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] python: fix Popen.wait() deadlock issue

Samba - samba-technical mailing list
Hi everyone,
I am Joe Guo, a new Samba developer from Catalyst IT, New Zealand.

In `python/samba/tests/__init__.py` file,  both `check_exit_code` and
`check_output` used `Popen.wait()` for command line output.
This will cause deadlock if the output is too large (for example, more
than 64k).

The issue is documented in Python official document at:
https://docs.python.org/2/library/subprocess.html#subprocess.Popen.wait

I created 2 patches for this issue.
0001 is the proof.  I added a python script to generate large output and
a known failed test case.
0002 is the fix. I use `communicate` instead of `wait` to avoid the
deadlock issue.


Cheers,
Joe Guo

0001-python-add-a-failed-test-to-show-Popen-deadlock.patch (7K) Download Attachment
0002-python-use-communicate-to-fix-Popen-deadlock.patch (2K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] python: fix Popen.wait() deadlock issue

Samba - samba-technical mailing list
On Tue, 2017-10-03 at 10:06 +1300, joeg--- via samba-technical wrote:

> Hi everyone,
> I am Joe Guo, a new Samba developer from Catalyst IT, New Zealand.
>
> In `python/samba/tests/__init__.py` file,  both `check_exit_code` and
> `check_output` used `Popen.wait()` for command line output.
> This will cause deadlock if the output is too large (for example, more
> than 64k).
>
> The issue is documented in Python official document at:
> https://docs.python.org/2/library/subprocess.html#subprocess.Popen.wait
>
> I created 2 patches for this issue.
> 0001 is the proof.  I added a python script to generate large output and
> a known failed test case.
> 0002 is the fix. I use `communicate` instead of `wait` to avoid the
> deadlock issue.
>

Thanks so much for this Joe!

Reviewed-by: Andrew Bartlett <[hidden email]>

Can I please get another team reviewer please?  This bit us recently
with a never-ending autobuild on sn-devel so it would be good to get
fixed.

Thanks,

Andrew Bartlett
--
Andrew Bartlett                       http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba