Patchwork [5,of,7] get-with-headers: handle broken pipes (py3)

login
register
mail settings
Submitter timeless
Date May 11, 2016, 5:20 a.m.
Message ID <67542cf802e8669a1c95.1462944027@gcc2-power8.osuosl.org>
Download mbox | patch
Permalink /patch/14996/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

timeless - May 11, 2016, 5:20 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1462561565 0
#      Fri May 06 19:06:05 2016 +0000
# Node ID 67542cf802e8669a1c954c3066f468fe3ae6ba0e
# Parent  3572b4c6d058583392f19d3c397804bc37e98a7b
# EXP-Topic runtests
# Available At bb://timeless/mercurial-crew
#              hg pull bb://timeless/mercurial-crew -r 67542cf802e8
get-with-headers: handle broken pipes (py3)

http://bugs.python.org/issue11380,#msg248579

indicates a recommended sequence for handling shutdown
Yuya Nishihara - May 16, 2016, 2:29 p.m.
On Wed, 11 May 2016 05:20:27 +0000, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1462561565 0
> #      Fri May 06 19:06:05 2016 +0000
> # Node ID 67542cf802e8669a1c954c3066f468fe3ae6ba0e
> # Parent  3572b4c6d058583392f19d3c397804bc37e98a7b
> # EXP-Topic runtests
> # Available At bb://timeless/mercurial-crew
> #              hg pull bb://timeless/mercurial-crew -r 67542cf802e8
> get-with-headers: handle broken pipes (py3)
> 
> http://bugs.python.org/issue11380,#msg248579

I read this issue, but I couldn't figure out why the problem appears only
on Python 3. Because get-with-headers.py doesn't handle exceptions, I expect
some errors would be displayed on Python 2.

Do you have any idea?
timeless - May 16, 2016, 6:06 p.m.
Yuya Nishihara wrote:

> I read this issue, but I couldn't figure out why the problem appears only
> on Python 3. Because get-with-headers.py doesn't handle exceptions, I
> expect
> some errors would be displayed on Python 2.
>

Well, the thread clearly indicates that the behavior seems to have changed
a couple of times.

I think that py3 tightened up the error reporting for writing to closed
pipes.

Beyond that, I really don't know. And I decided it wasn't worth my time to
try to investigate.

It doesn't help that different ways of piping limited amounts of characters
result in different interrupt behaviors...

We probably will want to update *all* of our code to use this rather long
and annoying chain. I'm hoping we could hide this with a context manager or
at least a function, because this nested stuff is really ugly.
Yuya Nishihara - May 17, 2016, 1:15 p.m.
On Mon, 16 May 2016 11:06:39 -0700, timeless wrote:
> Yuya Nishihara wrote:
> > I read this issue, but I couldn't figure out why the problem appears only
> > on Python 3. Because get-with-headers.py doesn't handle exceptions, I
> > expect
> > some errors would be displayed on Python 2.
> >  
> 
> Well, the thread clearly indicates that the behavior seems to have changed
> a couple of times.

Yes.

% python-2.4.6 -c 'print("hello")' | false
close failed: [Errno 32] Broken pipe

% python-2.6.9 -c 'print("hello")' | false 
close failed in file object destructor:
Error in sys.excepthook:

Original exception was:

% python-2.7.8 -c 'print("hello")' | false 
close failed in file object destructor:
sys.excepthook is missing
lost sys.stderr

% python3.5 -c 'print("hello")' | false
Exception ignored in: <_io.TextIOWrapper name='<stdout>' mode='w' encoding='UTF-8'>
BrokenPipeError: [Errno 32] Broken pipe

> I think that py3 tightened up the error reporting for writing to closed
> pipes.

My understanding is that all versions say something for broken pipe.

> Beyond that, I really don't know. And I decided it wasn't worth my time to
> try to investigate.

I agree.

I read this patch, and I thought you've hit the problem regularly on Python 3,
but I couldn't figure out why. It would be helpful if the commit message
contained the background of this patch.

> It doesn't help that different ways of piping limited amounts of characters
> result in different interrupt behaviors...
> 
> We probably will want to update *all* of our code to use this rather long
> and annoying chain. I'm hoping we could hide this with a context manager or
> at least a function, because this nested stuff is really ugly.

Hmm, I'm not sure. hg generally does flush() before exiting and catch EPIPE.

Patch

diff -r 3572b4c6d058 -r 67542cf802e8 tests/get-with-headers.py
--- a/tests/get-with-headers.py	Wed May 11 01:56:59 2016 +0000
+++ b/tests/get-with-headers.py	Fri May 06 19:06:05 2016 +0000
@@ -69,10 +69,22 @@ 
 
     return response.status
 
-status = request(sys.argv[1], sys.argv[2], sys.argv[3:])
-if twice:
+try:
     status = request(sys.argv[1], sys.argv[2], sys.argv[3:])
+    if twice:
+        status = request(sys.argv[1], sys.argv[2], sys.argv[3:])
 
-if 200 <= status <= 305:
-    sys.exit(0)
+    if 200 <= status <= 305:
+        sys.exit(0)
+finally:
+    try:
+        sys.stdout.flush()
+    finally:
+        try:
+            sys.stdout.close()
+        finally:
+            try:
+                sys.stderr.flush()
+            finally:
+                sys.stderr.close()
 sys.exit(1)