Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.debian.bugs.dist > #882694 > unrolled thread

Bug#890901: multiple pv -c : terminal left in -icanon -echo tostop

Started byLars Ellenberg <lars.ellenberg@linbit.com>
First post2018-02-20 13:50 +0100
Last post2018-02-20 19:00 +0100
Articles 2 — 2 participants

Back to article view | Back to linux.debian.bugs.dist


Contents

  Bug#890901: multiple pv -c : terminal left in -icanon -echo tostop Lars Ellenberg <lars.ellenberg@linbit.com> - 2018-02-20 13:50 +0100
    Bug#890901: multiple pv -c : terminal left in -icanon -echo tostop Andrew Wood <andrew.wood@ivarch.com> - 2018-02-20 19:00 +0100

#882694 — Bug#890901: multiple pv -c : terminal left in -icanon -echo tostop

FromLars Ellenberg <lars.ellenberg@linbit.com>
Date2018-02-20 13:50 +0100
SubjectBug#890901: multiple pv -c : terminal left in -icanon -echo tostop
Message-ID<vlfwR-DM-9@gated-at.bofh.it>
Package: pv
Version: 1.6.6-1
Severity: normal

Dear Maintainer,

On almost every try,
  seq 200 | pv -abc -N A | pv -abc -N B | pv -abc -N C > /dev/null
leaves my terminal with echo disabled (no reaction to typing).

It can be restored by blindly typing "reset" and/or "stty sane".

This happens because of the following race condition:

The first "pv" to start stores current termios in its t_save,
then changes it (disables echo).
The next "pv" stores that already changed termios in its t_save.

The last one to terminate "restores" to its t_save,
which can very well have echo disabled. D'uh.

debian testing pv 1.6.6-1
(which looks like it is basically the latest upstream)

reproducing "single" bash line above,
slightly more elaborate here,
which also restores the termios settings in each iteration:

# btw, for me stty -g is:
# 4500:5:bf:8a3b:3:1c:7f:15:4:0:1:0:11:13:1a:0:12:f:17:16:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0
stty_g=$(stty -g);
i=0;
while :; do
	printf "=== iteration: %3u\n" $(( ++i ));
	stty $stty_g;
	before=$(stty -a);
	seq 200 | pv -abc -N A | pv -abc -N B | pv -abc -N C > /dev/null;
	after=$(stty -a);
	stty $stty_g ;
	diff -U0 <(printf "%s\n" $before) <(printf "%s\n" $after) |
		grep -v '^@' |
		tee /dev/tty |
		grep -C10 echo && break;
done

For me, it usually takes under 10 iterations,
where "tostop" may remain earlier already,
but eventually even "-echo -icanon" remains :-(

=== iteration:   1
        A:  692 B [17,4MiB/s]
        C:  692 B [1,59MiB/s]
--- /dev/fd/63  2018-02-10 18:18:50.297415510 +0100
+++ /dev/fd/62  2018-02-10 18:18:50.297415510 +0100
-icanon
+-icanon
-echo
+-echo
--tostop
+tostop


A likely more real-life reproducer is a pipe like
 ... pv -abc -N in | xz -T2 -9 | pv -abc -N out ...

Maybe this could be fixed by storing not only the "crs_y_top",
but also "t_save" in IPC shm (from the first pv, the one that
initializes that shm segment while holding the tty lock),
so the last one to terminate can restore the termios settings
as found by the first one to start?

But that
 (lock;
   shm create and/or attach;
   if creator
      tcgetattr and save in shm,
   else load from shm,
      to be able to restore
      should I be the last one to leave
 unlock)
would need to happen very early. I think pv_crs_ipcinit() is currently
called too late, termios settings may have been changed already.

Anyways, such command lines usually are scripted,
and a work-around exists: just enclose the pipe containing pv
into stty_g=$(stty -g); pipe | goes | here; stty $stty_g
as I did in my reproducer above,
so "don't panic" ;-)

But maybe you can find a nice place to add this in?
Or document the problem and the workaround?

Cheers,

    Lars

This mail was also sent
 To: pv@ivarch.com, andrew.wood@ivarch.com
 Date: Sat, 10 Feb 2018 19:07:42 +0100
 Subject: multiple pv -c : terminal left in -icanon -echo tostop
 Message-ID: <20180210180728.GH19196@soda.linbit>

[toc] | [next] | [standalone]


#882737

FromAndrew Wood <andrew.wood@ivarch.com>
Date2018-02-20 19:00 +0100
Message-ID<vlkmR-3JB-13@gated-at.bofh.it>
In reply to#882694
Hello

Sorry for the delayed response.  I have received your other emails, but I am
quite far behind with my mailbox.

I have successfully reproduced the bug you describe, using the command line
you have provided.

Your suggested solution of storing the initial terminal state in the shared
memory segment sounds like a good one.  I should also be able to use the
test scenario you provided to write an automated test for this problem.

Thanks for the detailed bug report.


On Tue, Feb 20, 2018 at 01:40:58PM +0100, Lars Ellenberg wrote:
> Package: pv
> Version: 1.6.6-1
> Severity: normal
> 
> Dear Maintainer,
> 
> On almost every try,
>   seq 200 | pv -abc -N A | pv -abc -N B | pv -abc -N C > /dev/null
> leaves my terminal with echo disabled (no reaction to typing).
> 
> It can be restored by blindly typing "reset" and/or "stty sane".
> 
> This happens because of the following race condition:
> 
> The first "pv" to start stores current termios in its t_save,
> then changes it (disables echo).
> The next "pv" stores that already changed termios in its t_save.
> 
> The last one to terminate "restores" to its t_save,
> which can very well have echo disabled. D'uh.
> 
> debian testing pv 1.6.6-1
> (which looks like it is basically the latest upstream)
> 
> reproducing "single" bash line above,
> slightly more elaborate here,
> which also restores the termios settings in each iteration:
> 
> # btw, for me stty -g is:
> # 4500:5:bf:8a3b:3:1c:7f:15:4:0:1:0:11:13:1a:0:12:f:17:16:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0:0
> stty_g=$(stty -g);
> i=0;
> while :; do
> 	printf "=== iteration: %3u\n" $(( ++i ));
> 	stty $stty_g;
> 	before=$(stty -a);
> 	seq 200 | pv -abc -N A | pv -abc -N B | pv -abc -N C > /dev/null;
> 	after=$(stty -a);
> 	stty $stty_g ;
> 	diff -U0 <(printf "%s\n" $before) <(printf "%s\n" $after) |
> 		grep -v '^@' |
> 		tee /dev/tty |
> 		grep -C10 echo && break;
> done
> 
> For me, it usually takes under 10 iterations,
> where "tostop" may remain earlier already,
> but eventually even "-echo -icanon" remains :-(
> 
> === iteration:   1
>         A:  692 B [17,4MiB/s]
>         C:  692 B [1,59MiB/s]
> --- /dev/fd/63  2018-02-10 18:18:50.297415510 +0100
> +++ /dev/fd/62  2018-02-10 18:18:50.297415510 +0100
> -icanon
> +-icanon
> -echo
> +-echo
> --tostop
> +tostop
> 
> 
> A likely more real-life reproducer is a pipe like
>  ... pv -abc -N in | xz -T2 -9 | pv -abc -N out ...
> 
> Maybe this could be fixed by storing not only the "crs_y_top",
> but also "t_save" in IPC shm (from the first pv, the one that
> initializes that shm segment while holding the tty lock),
> so the last one to terminate can restore the termios settings
> as found by the first one to start?
> 
> But that
>  (lock;
>    shm create and/or attach;
>    if creator
>       tcgetattr and save in shm,
>    else load from shm,
>       to be able to restore
>       should I be the last one to leave
>  unlock)
> would need to happen very early. I think pv_crs_ipcinit() is currently
> called too late, termios settings may have been changed already.
> 
> Anyways, such command lines usually are scripted,
> and a work-around exists: just enclose the pipe containing pv
> into stty_g=$(stty -g); pipe | goes | here; stty $stty_g
> as I did in my reproducer above,
> so "don't panic" ;-)
> 
> But maybe you can find a nice place to add this in?
> Or document the problem and the workaround?
> 
> Cheers,
> 
>     Lars
> 
> This mail was also sent
>  To: pv@ivarch.com, andrew.wood@ivarch.com
>  Date: Sat, 10 Feb 2018 19:07:42 +0100
>  Subject: multiple pv -c : terminal left in -icanon -echo tostop
>  Message-ID: <20180210180728.GH19196@soda.linbit>

-- 
Andrew Wood

[toc] | [prev] | [standalone]


Back to top | Article view | linux.debian.bugs.dist


csiph-web