chgserver: discard buffered output before restoring fds (issue6207) stable
authorYuya Nishihara <yuya@tcha.org>
Mon, 20 Jul 2020 20:31:24 +0900
branchstable
changeset 45185 a17454a189d1
parent 45184 3781e9f74b27
child 45186 a52bf967e90a
chgserver: discard buffered output before restoring fds (issue6207) On Python 3, flush() appears not discarding buffered data on EPIPE, and the buffered data will be carried over to the restored stdout.
mercurial/chgserver.py
tests/test-chg.t
--- a/mercurial/chgserver.py	Tue Jul 21 20:49:05 2020 +0900
+++ b/mercurial/chgserver.py	Mon Jul 20 20:31:24 2020 +0900
@@ -434,8 +434,11 @@
             self._oldios.append((ch, fp, fd))
 
     def _restoreio(self):
+        if not self._oldios:
+            return
+        nullfd = os.open(os.devnull, os.O_WRONLY)
         ui = self.ui
-        for (ch, fp, fd), (cn, fn, _mode) in zip(self._oldios, _iochannels):
+        for (ch, fp, fd), (cn, fn, mode) in zip(self._oldios, _iochannels):
             newfp = getattr(ui, fn)
             # close newfp while it's associated with client; otherwise it
             # would be closed when newfp is deleted
@@ -443,6 +446,12 @@
                 newfp.close()
             # restore original fd: fp is open again
             try:
+                if newfp is fp and 'w' in mode:
+                    # Discard buffered data which couldn't be flushed because
+                    # of EPIPE. The data should belong to the current session
+                    # and should never persist.
+                    os.dup2(nullfd, fp.fileno())
+                    fp.flush()
                 os.dup2(fd, fp.fileno())
             except OSError as err:
                 # According to issue6330, running chg on heavy loaded systems
@@ -459,6 +468,7 @@
             os.close(fd)
             setattr(self, cn, ch)
             setattr(ui, fn, fp)
+        os.close(nullfd)
         del self._oldios[:]
 
     def validate(self):
--- a/tests/test-chg.t	Tue Jul 21 20:49:05 2020 +0900
+++ b/tests/test-chg.t	Mon Jul 20 20:31:24 2020 +0900
@@ -152,6 +152,49 @@
   crash-pager: going to crash
   [255]
 
+no stdout data should be printed after pager quits, and the buffered data
+should never persist (issue6207)
+
+"killed!" may be printed if terminated by SIGPIPE, which isn't important
+in this test.
+
+  $ cat > $TESTTMP/bulkwrite.py <<'EOF'
+  > import time
+  > from mercurial import error, registrar
+  > cmdtable = {}
+  > command = registrar.command(cmdtable)
+  > @command(b'bulkwrite')
+  > def bulkwrite(ui, repo, *pats, **opts):
+  >     ui.write(b'going to write massive data\n')
+  >     ui.flush()
+  >     t = time.time()
+  >     while time.time() - t < 2:
+  >         ui.write(b'x' * 1023 + b'\n')  # will be interrupted by SIGPIPE
+  >     raise error.Abort(b"write() doesn't block")
+  > EOF
+
+  $ cat > $TESTTMP/fakepager.py <<'EOF'
+  > import sys
+  > import time
+  > sys.stdout.write('paged! %r\n' % sys.stdin.readline())
+  > time.sleep(1)  # new data will be written
+  > EOF
+
+  $ cat >> .hg/hgrc <<EOF
+  > [extensions]
+  > bulkwrite = $TESTTMP/bulkwrite.py
+  > EOF
+
+  $ chg bulkwrite --pager=on --color no --config ui.formatted=True
+  paged! 'going to write massive data\n'
+  killed! (?)
+  [255]
+
+  $ chg bulkwrite --pager=on --color no --config ui.formatted=True
+  paged! 'going to write massive data\n'
+  killed! (?)
+  [255]
+
   $ cd ..
 
 server lifecycle