Patchwork [8,of,9,v2] stdio: raise StdioError if something goes wrong in ui.flush

login
register
mail settings
Submitter Bryan O'Sullivan
Date April 14, 2017, 4:29 a.m.
Message ID <800f9ec2664e515d8ad8.1492144186@jsdf-mbp.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/20191/
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 800f9ec2664e515d8ad807c0df1efaccda8ff5e5
# Parent  fa74f393eaa5663b943eb0544ad4723ae7371385
stdio: raise StdioError if something goes wrong in ui.flush

The prior code used to ignore all errors, which was intended to
deal with a decade-old problem with writing to broken pipes on
Windows.

However, that code inadvertantly went a lot further, making it
impossible to detect *all* I/O errors on stdio ... but only sometimes.

What actually happened was that if Mercurial wrote less than a stdio
buffer's worth of output (the overwhelmingly common case for most
commands), any error that occurred would get swallowed here.  But
if the buffering strategy changed, an unhandled IOError could be
raised from any number of other locations.

Because we now have a top-level StdioError handler, and ui._write
and ui._write_err (and now flush!) will raise that exception, we
have one rational place to detect and handle these errors.

Patch

diff --git a/mercurial/ui.py b/mercurial/ui.py
--- a/mercurial/ui.py
+++ b/mercurial/ui.py
@@ -807,10 +807,15 @@  class ui(object):
         # opencode timeblockedsection because this is a critical path
         starttime = util.timer()
         try:
-            try: self.fout.flush()
-            except (IOError, ValueError): pass
-            try: self.ferr.flush()
-            except (IOError, ValueError): pass
+            try:
+                self.fout.flush()
+            except IOError as err:
+                raise error.StdioError(err)
+            finally:
+                try:
+                    self.ferr.flush()
+                except IOError as err:
+                    raise error.StdioError(err)
         finally:
             self._blockedtimes['stdio_blocked'] += \
                 (util.timer() - starttime) * 1000