Re: process substitution fd lifetime race condition

2020-04-20 Thread Chet Ramey
On 4/20/20 1:15 AM, Jason A. Donenfeld wrote:
> Hi,
> 
> I've uncovered a very unusual race condition when using process
> substitution and developed as minimal a reproducer as I could create:

What version of bash are you using?

Chet

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: process substitution fd lifetime race condition

2020-04-20 Thread Jason A. Donenfeld
On 4/20/20, Chet Ramey  wrote:
> On 4/20/20 1:15 AM, Jason A. Donenfeld wrote:
>> Hi,
>>
>> I've uncovered a very unusual race condition when using process
>> substitution and developed as minimal a reproducer as I could create:
>
> What version of bash are you using?
>

5.0.016



Re: process substitution fd lifetime race condition

2020-04-20 Thread Jason A. Donenfeld
Here's a simpler reproducer:

set -e
a="my name is a"
b="my name is b"
pretty() { echo -e "\x1b[0m"; }
doit() { pretty; "$@"; }
while true; do
   doit cat <(echo "$a") <(echo "$b")
done



Re: process substitution fd lifetime race condition

2020-04-20 Thread Jason A. Donenfeld
This one will reproduce immediately:

#!/bin/bash
set -e
a="my name is a"
b="my name is b"
sleep() { read -t "$1" -N 1 || true; }
doit() { sleep 0.1; "$@"; }
while true; do
   doit cat <(echo "$a") <(echo "$b")
done



Re: process substitution fd lifetime race condition

2020-04-20 Thread Chet Ramey
On 4/20/20 5:01 PM, Jason A. Donenfeld wrote:
> On 4/20/20, Chet Ramey  wrote:
>> On 4/20/20 1:15 AM, Jason A. Donenfeld wrote:
>>> Hi,
>>>
>>> I've uncovered a very unusual race condition when using process
>>> substitution and developed as minimal a reproducer as I could create:
>>
>> What version of bash are you using?
>>
> 
> 5.0.016

See if the patch in this message helps things. I'm skeptical, but give it
a try.

https://lists.gnu.org/archive/html/bug-bash/2020-04/msg00018.html

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: process substitution fd lifetime race condition

2020-04-20 Thread Jason A. Donenfeld
On Mon, Apr 20, 2020 at 3:49 PM Chet Ramey  wrote:
>
> On 4/20/20 5:01 PM, Jason A. Donenfeld wrote:
> > On 4/20/20, Chet Ramey  wrote:
> >> On 4/20/20 1:15 AM, Jason A. Donenfeld wrote:
> >>> Hi,
> >>>
> >>> I've uncovered a very unusual race condition when using process
> >>> substitution and developed as minimal a reproducer as I could create:
> >>
> >> What version of bash are you using?
> >>
> >
> > 5.0.016
>
> See if the patch in this message helps things. I'm skeptical, but give it
> a try.
>
> https://lists.gnu.org/archive/html/bug-bash/2020-04/msg00018.html

Actually, it seems to work indeed... So far I can't repro with that.



Re: process substitution fd lifetime race condition

2020-04-20 Thread Chet Ramey
On 4/20/20 5:56 PM, Jason A. Donenfeld wrote:
> On Mon, Apr 20, 2020 at 3:49 PM Chet Ramey  wrote:
>>
>> On 4/20/20 5:01 PM, Jason A. Donenfeld wrote:
>>> On 4/20/20, Chet Ramey  wrote:
 On 4/20/20 1:15 AM, Jason A. Donenfeld wrote:
> Hi,
>
> I've uncovered a very unusual race condition when using process
> substitution and developed as minimal a reproducer as I could create:

 What version of bash are you using?

>>>
>>> 5.0.016
>>
>> See if the patch in this message helps things. I'm skeptical, but give it
>> a try.
>>
>> https://lists.gnu.org/archive/html/bug-bash/2020-04/msg00018.html
> 
> Actually, it seems to work indeed... So far I can't repro with that.

OK, good. It was either that or closing the fd after reaping the child
process -- I couldn't tell 100% from the system call trace.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: process substitution fd lifetime race condition

2020-04-20 Thread Jason A. Donenfeld
On Mon, Apr 20, 2020 at 3:58 PM Chet Ramey  wrote:
> OK, good. It was either that or closing the fd after reaping the child
> process -- I couldn't tell 100% from the system call trace.

The latter is an interesting possibility. I assume SIGCHLD comes in
through a signal handler, so it's asynchronous, which means it could
race. But in the case that fd 63 hasn't already been closed, wont
subsequent fifos use different fds, which means the signal handler
doesn't need to synchronize with anything?

Either way, glad to hear that your patch fixes the issue. Looking
forward to the release of 5.0.017.

It seems like process substitution fifo lifetime is really tricky. You
can't really reference track, since the path is just a string that
could be manipulated. So how do you know when it's safe to clean up
that fd and that nobody is using it? I suppose you have a set of
lexical heuristics that fit for "most common cases"? And I guess those
are undocumented and can't really be relied upon since evidently they
do change.

I wonder if one general improvement would be to never close a fifo if
it has data in it, but it's read position is still at zero.



Re: process substitution fd lifetime race condition

2020-04-20 Thread Chet Ramey
On 4/20/20 6:02 PM, Jason A. Donenfeld wrote:

> It seems like process substitution fifo lifetime is really tricky. You
> can't really reference track, since the path is just a string that
> could be manipulated. So how do you know when it's safe to clean up
> that fd and that nobody is using it? 

In general, you clean up a procsub fd, and remove any named pipes from the
file system, when the block that uses it (or created it) ends. That is, as
you noticed, sometimes tricky.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/



Re: process substitution fd lifetime race condition

2020-04-20 Thread Jason A. Donenfeld
Were you planning on committing this to Savannah?



Re: process substitution fd lifetime race condition

2020-04-21 Thread Chet Ramey
On 4/21/20 12:29 AM, Jason A. Donenfeld wrote:
> Were you planning on committing this to Savannah?

Yes, this will most likely end up as patch 17, unless a higher-priority
thing comes up first.

-- 
``The lyf so short, the craft so long to lerne.'' - Chaucer
 ``Ars longa, vita brevis'' - Hippocrates
Chet Ramey, UTech, CWRUc...@case.eduhttp://tiswww.cwru.edu/~chet/