I cannot reproduce. My "hg debuginstall" is the same. Do you have any special pager configuration?

(In reply to Denis Laxalde from comment #1)

My normal pager config is:
~$ hg config pager
pager.pager=less -FRX

I did this in /tmp with HOME=$PWD, any pager configuration would be the defaults.

hg$ hg config pager
pager.pager=sensible-pager

~$ env|grep LESS
LESS=FXR
LESSCLOSE=/usr/bin/lesspipe %s %s
LESSOPEN=| /usr/bin/lesspipe %s

Still happens if I unset those variables.

Terminal is xfce4-terminal on Debian 10.
Happens with st (stterm) too.
Not sure what else might affect this. The main change was in Python version.

Can reproduce on Ubuntu 18.04 with Python 3.6.

$ hg debuginstall
checking encoding (UTF-8)...
checking Python executable (/home/ian/hgpy3/bin/python3)
checking Python version (3.6.8)
checking Python lib (/home/ian/hgpy3/lib/python3.6)...
checking Python security support (sni,tls1.0,tls1.1,tls1.2)
checking Mercurial version (5.2rc0)
checking Mercurial custom build ()
checking module policy (c)
checking installed modules (/home/ian/hgpy3/lib/python3.6/site-packages/mercurial)...
checking registered compression engines (bz2, bz2truncated, none, zlib, zstd)
checking available compression engines (bz2, bz2truncated, none, zlib, zstd)
checking available compression engines for wire protocol (zstd, zlib, bz2, none)
checking "re2" regexp engine (missing)
checking templates (/home/ian/hgpy3/lib/python3.6/site-packages/mercurial/templates)...
checking default template (/home/ian/hgpy3/lib/python3.6/site-packages/mercurial/templates/map-cmdline.default)
checking commit editor... (subl)
checking username (Ian Moody <[redacted]>)
no problems detected

$ hg config pager
pager.pager=less -EFRSXQ

This is with chg (I had forgotten my hg=chg alias)

It behaves as expected when running the real hg.

Ok, confirmed. I can reproduce with chg.

Bug was inactive for 150 days, archiving

The bug still exists. I’m currently writing a detailed bug analysis. A little spoiler: The bug happens even in the non-chg case, but it’s masked by another bug. With the attached diff bug-non-chg.diff, you can observe a similar error without chg.

Created attachment 2081 [details]
trigger bug even in the non-chg case

Mercurial writes it output to stream objects (Python 2’s `file` objects, wrapping libc’s FILE objects, and Python 3’s own implementation in the `io` module). When a pager is started, the FDs underlying these stream objects are redirected (using dup2()) to the writing end of a pipe whose reading side is the pager’s stdin. There are three levels on which buffering happens. First, the (user-space) stream objects are buffered (usually line-buffered), the pipes are buffered in the kernel, and the pager might do some buffering. If the pipe gets full (because the pager doesn’t read enough data), the write() system call blocks.

When the pager exits, the pipe breaks, causing a SIGPIPE signal and that the write() system call exits. The (kernel-space) pipe gets abandoned. What happens with the buffered bytes in the stream objects is not well-defined. Glibc’s stream objects seem to discard them when encountering a broken pipe. Python 3’s stream objects don’t discard them. Everytime the flush() method gets called, it will try to write the buffered bytes to the underlying FD.

By accident, the user-visible bug doesn’t happen in the non-chg case. The finally clause in `mercurial.dispatch.dispatch()` calls `req.ui.flush()`. On Python 3, this will try to write to the broken pipe again. An IOError with EPIPE is explicitly silenced in `mercurial.ui.flush()`. However, before the pager was started, the SIGPIPE signal was changed to raise `mercurial.error.SignalInterrupt`, which is not silenced. Therefore the exception bubbles up, causing `dispatch()` to not run the exit handlers. How we should handle this sitation is maybe dependent on how we decide to solve the user-visible bug, so I suggest to postpone this question.

The `killpager()` function in `mercurial.ui` is such an exit handler and will redirect the stdout and stderr FDs back to the original files. Chg handles this otherwise (therefore it’s not affected by the bug described in the previous paragraph). When redirecting the underlying FD to a different file than the broken pipe, both Glibc and Python 3 continue to work.

An indeterminate number of bytes was written by Mercurial at this point (depending on how much the pager read, how much the kernel buffered, and how much the stream objects buffered). The bytes written to the pipe get lost in any case. On Python 2, more bytes (submitted to the libc, but not written to the pipe) could get lost. Therefore, there’s a high chance that some bytes are lost. When continuing to write, there would be a hole of indeterminate size in between. In the case of ui.fout, I think it’s unacceptable if bytes in the middle of the stream get lost, as it harms the integrity of the output. We should therefore close ui.fout when encountering a broken pipe. For ui.ferr, it’s not as clear. Incomplete error messages could be misleading, but overall it’s probably better to not drop error messages.

Yuya, I’d appreciate your opinion on this.

As far as I can see, the data is flushed to the stdout of the server process,
not to the client. So the simplest fix would be to discard the buffered data
before restoring the original fd.

> close ui.fout when encountering a broken pipe.

Closing stdout wouldn't be nice for chg, which heavly depends on os.dup2().

> overall it’s probably better to not drop error messages.

Maybe, but in order to make both plain hg and chg behave exactly the same,
we'll need some more dup2() work. chg doesn't backup the client's stdout/err fd.
I think it's fine to discard both stdout/err contents on EPIPE.

diff --git a/mercurial/chgserver.py b/mercurial/chgserver.py
--- a/mercurial/chgserver.py
+++ b/mercurial/chgserver.py
@@ -434,8 +434,9 @@ class chgcmdserver(commandserver.server)
             self._oldios.append((ch, fp, fd))
 
     def _restoreio(self):
+        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 +444,12 @@ class chgcmdserver(commandserver.server)
                 newfp.close()
             # restore original fd: fp is open again
             try:
+                if newfp is fp and b'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 +466,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):

> the data is flushed to the stdout of the server process,

This appears wrong. It is the stdout when "runcommand" request is issued,
which I think is the client stdout.

Fixed by https://mercurial-scm.org/repo/hg/rev/a17454a189d1
Yuya Nishihara <yuya@tcha.org>
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.

(please test the fix)

Bug was set to TESTING for 7 days, resolving