Submitter | Yuya Nishihara |
---|---|
Date | July 21, 2020, 12:42 p.m. |
Message ID | <88f93fc8cb3263a3c101.1595335366@mimosa> |
Download | mbox | patch |
Permalink | /patch/46814/ |
State | Accepted |
Headers | show |
Comments
queued for stable, thanks > On Jul 21, 2020, at 08:42, Yuya Nishihara <yuya@tcha.org> wrote: > > # HG changeset patch > # User Yuya Nishihara <yuya@tcha.org> > # Date 1595244684 -32400 > # Mon Jul 20 20:31:24 2020 +0900 > # Branch stable > # Node ID 88f93fc8cb3263a3c10143557cb9626dfcf23f4c > # Parent f91f0dfccf9b0a928e22bf0041b310712c93903c > 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. > > diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py > --- a/mercurial/chgserver.py > +++ b/mercurial/chgserver.py > @@ -434,8 +434,11 @@ class chgcmdserver(commandserver.server) > 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 @@ class chgcmdserver(commandserver.server) > 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 @@ class chgcmdserver(commandserver.server) > os.close(fd) > setattr(self, cn, ch) > setattr(ui, fn, fp) > + os.close(nullfd) > del self._oldios[:] > > def validate(self): > diff --git a/tests/test-chg.t b/tests/test-chg.t > --- a/tests/test-chg.t > +++ b/tests/test-chg.t > @@ -152,6 +152,49 @@ chg waits for pager if runcommand raises > 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 > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Patch
diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py --- a/mercurial/chgserver.py +++ b/mercurial/chgserver.py @@ -434,8 +434,11 @@ class chgcmdserver(commandserver.server) 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 @@ class chgcmdserver(commandserver.server) 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 @@ class chgcmdserver(commandserver.server) os.close(fd) setattr(self, cn, ch) setattr(ui, fn, fp) + os.close(nullfd) del self._oldios[:] def validate(self): diff --git a/tests/test-chg.t b/tests/test-chg.t --- a/tests/test-chg.t +++ b/tests/test-chg.t @@ -152,6 +152,49 @@ chg waits for pager if runcommand raises 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