a-j-wood/pv

Race condition with multiple "pv -c" leaves terminal state inconsistent

Closed this issue · 1 comments

Date: Tue, 20 Feb 2018 13:40:58 +0100
From: Lars Ellenberg
To: Debian Bug Tracking System
Subject: Bug#890901: multiple pv -c : terminal left in -icanon -echo tostop

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

Another similar report from Viktor Ashirov

On Sun, Sep 29, 2019 at 02:01:57PM +0200, Viktor Ashirov wrote:

Hi Andrew,

Thank you for creating 'pv', I find it very helpful!

I found an issue with 'pv'. After running the following reproducer,
terminal ends up without echo:

dd if=/dev/zero bs=1M count=1024 | pv -cN download | pv -cN unpack |
dd of=/dev/null

$ stty -a | grep echo; dd if=/dev/zero bs=1M count=1024 | pv -cN
download | pv -cN unpack | dd of=/dev/null; stty -a | grep echo
isig icanon iexten echo echoe echok -echonl -noflsh -xcase tostop -echoprt
echoctl echoke -flusho -extproc
download: 1GiB 0:00:03 [ 320MiB/s] [ <=> ]
unpack: 1GiB 0:00:03 [ 320MiB/s] [ <=> ]
2097152+0 records in
2097152+0 records out GB, 1.0 GiB) copied, 3.19649 s, 336 MB/s
1073741824 bytes (1.1 GB, 1.0 GiB) copied, 3.19742 s, 336 MB/s
isig -icanon iexten -echo echoe echok -echonl -noflsh -xcase tostop -echoprt
echoctl echoke -flusho -extproc

It happens on both bash and zsh, and I tested pv versions from 1.4.0
to 1.6.6. I suspect that running two copies of 'pv -c' might cause a
race condition.
Please let me know if you need any additional information.

Regards,
Viktor