changeset 48185:2f2107c01dee

dispatch: ignore failure to flush ui When the pager dies, we get a `SIGPIPE`. That causes `error.SignalInterrupt` to be raised ` (from `ui._catchterm()`). Any further writes or flushes will cause further `SIGPIPE`s and furhter `error.SignalInterrupt`. If we write or flush outside of the try/except that handle `KeyboardInterrupt` (which `error.SignalInterrupt` is a subclass of), then control will escape from the `dispatch` module. Let's fix that by ignoring errors from flushing the ui. I would have rather fixed this by restoring the stdout and stderr streams when the pager dies, but it gets complicated because of multiple ui instances (ui/lui) and different pager setups between regular hg and chg. This changes a test in `test-pager.t`, but I don't understand why. I would have thought that all the output from the command should have gone to the broken pager. Differential Revision: https://phab.mercurial-scm.org/D11627
author Martin von Zweigbergk <martinvonz@google.com>
date Fri, 08 Oct 2021 13:36:02 -0700
parents 8fae2cc6ee86
children 6edc8800dbc3
files mercurial/dispatch.py tests/test-pager.t
diffstat 2 files changed, 21 insertions(+), 13 deletions(-) [+]
line wrap: on
line diff
--- a/mercurial/dispatch.py	Fri Oct 08 13:34:33 2021 -0700
+++ b/mercurial/dispatch.py	Fri Oct 08 13:36:02 2021 -0700
@@ -252,9 +252,14 @@
         err = e
         status = -1
 
-    ret = _flushstdio(req.ui, err)
-    if ret and not status:
-        status = ret
+    # Somehow we have to catcht he exception here; catching it inside
+    # _flushstdio() doesn't work.
+    try:
+        ret = _flushstdio(req.ui, err)
+        if ret and not status:
+            status = ret
+    except BaseException:
+        pass
     return status
 
 
@@ -314,7 +319,10 @@
             ret = -1
         finally:
             duration = util.timer() - starttime
-            req.ui.flush()  # record blocked times
+            try:
+                req.ui.flush()  # record blocked times
+            except BaseException:
+                pass
             if req.ui.logblockedtimes:
                 req.ui._blockedtimes[b'command_duration'] = duration * 1000
                 req.ui.log(
@@ -338,7 +346,10 @@
             except:  # exiting, so no re-raises
                 ret = ret or -1
             # do flush again since ui.log() and exit handlers may write to ui
-            req.ui.flush()
+            try:
+                req.ui.flush()
+            except BaseException:
+                pass
         return ret
 
 
@@ -459,7 +470,10 @@
                 try:
                     return _dispatch(req)
                 finally:
-                    ui.flush()
+                    try:
+                        ui.flush()  # record blocked times
+                    except BaseException:
+                        pass
             except:  # re-raises
                 # enter the debugger when we hit an exception
                 if req.earlyoptions[b'debugger']:
--- a/tests/test-pager.t	Fri Oct 08 13:34:33 2021 -0700
+++ b/tests/test-pager.t	Fri Oct 08 13:36:02 2021 -0700
@@ -219,8 +219,7 @@
 #endif
 
 A complicated pager command gets worse behavior. Bonus points if you can
-improve this. Windows apparently does this better, but only sometimes?
-#if windows
+improve this.
   $ hg log --limit 3 \
   >   --config pager.pager='this-command-better-never-exist --seriously' \
   >  2>/dev/null || true
@@ -240,11 +239,6 @@
   date:        Thu Jan 01 00:00:00 1970 +0000 (?)
   summary:     modify a 8 (?)
    (?)
-#else
-  $ hg log --limit 3 \
-  >   --config pager.pager='this-command-better-never-exist --seriously' \
-  >  2>/dev/null || true
-#endif
 
 Pager works with shell aliases.