Patchwork [5,of,9,v2] stdio: catch StdioError in dispatch.run and clean up appropriately

login
register
mail settings
Submitter Bryan O'Sullivan
Date April 14, 2017, 4:29 a.m.
Message ID <942022da49166766fe4a.1492144183@jsdf-mbp.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/20188/
State Accepted
Headers show

Comments

Bryan O'Sullivan - April 14, 2017, 4:29 a.m.
# HG changeset patch
# User Bryan O'Sullivan <bryano@fb.com>
# Date 1491947652 25200
#      Tue Apr 11 14:54:12 2017 -0700
# Node ID 942022da49166766fe4a7967b71411879221c197
# Parent  a5aa1dfd9afed15c0cd762c4a72e5e0082ac074c
stdio: catch StdioError in dispatch.run and clean up appropriately

We attempt to report what went wrong, and more importantly exit the
program with an error code.

(The exception we catch is not yet raised anywhere in the code.)
Yuya Nishihara - April 15, 2017, 7:34 a.m.
On Thu, 13 Apr 2017 21:29:43 -0700, Bryan O'Sullivan wrote:
> # HG changeset patch
> # User Bryan O'Sullivan <bryano@fb.com>
> # Date 1491947652 25200
> #      Tue Apr 11 14:54:12 2017 -0700
> # Node ID 942022da49166766fe4a7967b71411879221c197
> # Parent  a5aa1dfd9afed15c0cd762c4a72e5e0082ac074c
> stdio: catch StdioError in dispatch.run and clean up appropriately
> 
> We attempt to report what went wrong, and more importantly exit the
> program with an error code.
> 
> (The exception we catch is not yet raised anywhere in the code.)
> 
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -77,7 +77,22 @@ class request(object):
>  
>  def run():
>      "run the command in sys.argv"
> -    sys.exit((dispatch(request(pycompat.sysargv[1:])) or 0) & 255)
> +    req = request(pycompat.sysargv[1:])
> +    err = None
> +    try:
> +        status = (dispatch(req) or 0) & 255
> +    except error.StdioError as err:
> +        status = -1
> +    if util.safehasattr(req.ui, 'fout'):
> +        try:
> +            req.ui.fout.close()
> +        except IOError as err:
> +            status = -1
> +    if util.safehasattr(req.ui, 'ferr'):
> +        if err is not None and err.errno != errno.EPIPE:
> +            req.ui.ferr.write('abort: %s\n' % err.strerror)
> +        req.ui.ferr.close()
> +    sys.exit(status & 255)

Can't we change .close() to .flush() ?

As I said before, req.ui.fout != sys.__stdout__ but they share the same fd.
If we had an extension that registers Python atexit handler to print something,
the Python process would crash on Windows.

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -77,7 +77,22 @@  class request(object):
 
 def run():
     "run the command in sys.argv"
-    sys.exit((dispatch(request(pycompat.sysargv[1:])) or 0) & 255)
+    req = request(pycompat.sysargv[1:])
+    err = None
+    try:
+        status = (dispatch(req) or 0) & 255
+    except error.StdioError as err:
+        status = -1
+    if util.safehasattr(req.ui, 'fout'):
+        try:
+            req.ui.fout.close()
+        except IOError as err:
+            status = -1
+    if util.safehasattr(req.ui, 'ferr'):
+        if err is not None and err.errno != errno.EPIPE:
+            req.ui.ferr.write('abort: %s\n' % err.strerror)
+        req.ui.ferr.close()
+    sys.exit(status & 255)
 
 def _getsimilar(symbols, value):
     sim = lambda x: difflib.SequenceMatcher(None, value, x).ratio()