leahneukirchen / mblaze

Unix utilities to deal with Maildir
Other
441 stars 48 forks source link

mpick: multi redir test hangs on Alpine #203

Closed nmeum closed 3 years ago

nmeum commented 3 years ago

While trying to get the mblaze test suite running on Alpine Linux, I noticed that the multi redir test from 2000-mpick.t does not terminate, even with current git HEAD. To reproduce:

$ cat /tmp/input
inbox/cur/1:2,S
inbox/cur/2:2,ST
inbox/cur/3:2,SRT
inbox/cur/4:2,SRFT
inbox/cur/5:2,T
inbox/cur/6:2,SRF
inbox/cur/7:2,SR
inbox/cur/8:2,SF
inbox/cur/9:2,
$ cat expr
let foo = from.addr == "peter@example.org"
let bar = from.disp == "Peter Example"
in
  foo |"sed ""s/^/1:&/""" && bar |"sed ""s/^/2:&/""" && skip
$ mpick -F ./expr < /tmp/input
1:inbox/cur/1:2,S
1:inbox/cur/9:2,
2:inbox/cur/1:2,S
2:inbox/cur/9:2,
<process does not terminate>

I debugged this a bit with strace(1). The output of the 3 processes (mpick, sed, and sed) is available here:

I am a bit confused as to what is happening here. The mpick process (PID 25493) blocks because it waits for the first sed process (PID 25494) to terminate (since mpick uses pclose(3)). The sed process on the other hand doesn't terminate because it is still reading from the reading end of the pipe. However, I would expect that read to return with an EOF because the mpick process already closed the writing end of the pipe (fd 4). Yet for some reason, it is stuck in a read system call. Maybe I am missing something obvious here, any idea what might be causing this?

Duncaen commented 3 years ago

This looks like a bug in musl, the second popen'ed process ends up inheriting the writing end of the first popen.

The popen() function shall ensure that any streams from previous popen() calls that remain open in the parent process are closed in the new child process. https://pubs.opengroup.org/onlinepubs/9699919799/

echo "Hello\nWorld" | busybox awk '{print $0 |"cat -n"; print $0 |"cat  -n"}'
nmeum commented 3 years ago

Ah, indeed. That would also explain why closing the pipes in reverse order fixes it. And fd 4 does also show up in /proc/259495/fd. I will report this upstream.

nmeum commented 3 years ago

I will report this upstream.

https://www.openwall.com/lists/musl/2021/03/14/1

nmeum commented 3 years ago

This seems to have been fixed recently on the musl master branch https://git.musl-libc.org/cgit/musl/commit/?id=e1a51185ceb4386481491e11f6dd39569b9e54f7 As such, the mblaze tests should pass with the upcoming musl release. I think we can close this now.