Control: tag -1 patch

On Tue, Apr 16, 2019 at 01:03:28PM -0400, Antoine Beaupré wrote:
 
> Okay, so here's an interesting data point. On Debian stretch, I can't
> reproduce the bug, even when running from git, using on current master
> (1.20180726-30-g6cf8003).
> 
> So maybe something else is going on here, outside of myrepos itself...

Hi, could you please recheck this data point? I'm seeing the buggy
behaviour on stretch too with mr from current sid (1.20180726, just
copying the sid version of /usr/bin/mr onto stretch system).

Maybe you didn't have libio-pty-easy-perl installed on the stretch system
as that would silently mask the issue?

As far as I can see, the problem here is that the 
  while (my $data = $pty->read()) { ... }
loop assumes EOF when $data is false, and doesn't check for undef. 
The IO::Pty::Easy docs say for read():

       Returns "undef" on timeout, the empty string on EOF, or a string
       of at least one character on success (this is consistent with
       "sysread()" and Term::ReadKey).

When SIGWINCH interrupts the blocking read(), mr proceeds to close the pty,
and the child processes die from SIGHUP.

So this doesn't look to me like a regression in Perl or other
dependencies, just a bug in the code.

Maybe something like the attached patch would do? Only lightly tested
but seems to fix it for me.
-- 
Niko Tyni   [email protected]
>From 0cf5cc08885dc65a0824f4f8d3409babe1ac5747 Mon Sep 17 00:00:00 2001
From: Niko Tyni <[email protected]>
Date: Mon, 14 Oct 2019 15:44:43 +0300
Subject: [PATCH] Fix signal handling in terminal_friendly_spawn()

When reading in blocking mode, we need to differentiate between EOF and
interruptions due to signals so we don't close the pty prematurely in
the latter case.

Closes: 927154
---
 mr | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/mr b/mr
index cf7cf3f..41073a3 100755
--- a/mr
+++ b/mr
@@ -865,7 +865,10 @@ sub terminal_friendly_spawn {
 			}
 		}
 		else {
-			while (my $data = $pty->read()) {
+			while (1) {
+				my $data = $pty->read();
+				next if !defined $data; # interrupted by a signal
+				last if $data eq '';    # EOF
 				print $output_cache $data if defined $output_cache;
 				$output .= $data;
 			}
-- 
2.23.0

Reply via email to