changeset 45185:a17454a189d1 stable

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.

--- 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