Re: [boost] [gsoc] Boost.Process done

Subject: Re: [boost] [gsoc] Boost.Process done
From: Ilya Sokolov (ilyasokol_at_[hidden])
Date: 2010-08-19 17:58:08

Boris Schaeling wrote:
> On Thu, 19 Aug 2010 21:17:08 +0200, Ilya Sokolov <ilyasokol_at_[hidden]> 
> wrote:
> 
>> Boris Schaeling wrote:
>>> On Wed, 18 Aug 2010 11:23:56 +0200, Ilya Sokolov 
>>> <ilyasokol_at_[hidden]> wrote:
>>>
>>>> [...]Now after line 3 the child process has the following fds:
>>>> 0    closed
>>>> 1    wend of pipe created for stdout
>>>> 2    wend of pipe created for stdout (!)
>>>> 3    closed
>>>  Not sure why you think so? I've attached the program I used to try 
>>> to reproduce the behavior but it works fine. Maybe you can update it 
>>> if you think there is a problem?
>>
>> Could you swap these lines?
>>
>> ctx.stdout_behavior = bpb::pipe::create(bpb::pipe::output_stream);
>> ctx.stderr_behavior = bpb::pipe::create(bpb::pipe::output_stream);
> 
> I see, very good catch! File descriptor 3 (the write end of the pipe 
> created for stdout) is mapped to STDOUT_FILENO with dup2(). As 
> STDOUT_FILENO is file descriptor 1 that one is automatically closed by 
> dup2(). In this use case file descriptor 1 is unfortunately the write 
> end of the pipe created for stderr though.

In other words, wend of pipe created for stdout overwrites wend of pipe
created for stderr.

> Any idea how to easily fix this? As far as I see the order of mapping 
> read/write ends to stdin, stdout and stderr depends on whether another 
> behavior actually uses the file descriptors 0, 1 and 2. In the worst 
> case however the write end of the pipe created for stdout is 2 and the 
> write end of the pipe created for stderr 1. Either we put a warning in 
> the documentation or we come up with some clever code? (In the moment 
> I'm a bit in a hurry. If there are no proposals I'll think about it on 
> the weekend. :)

Three choices that I know of:

1. Before creating any new file (such as in bpb::pipe::create()
or bpb::dummy::create(), not sure about bpb::inherit::create()), you 
could "occupy" fds which behavior has been set to != bpb::inherit.

2. Create new files in the ascending order of the corresponding fds.

3. Put this and other problems on the user, like in posix_spawn()
http://www.opengroup.org/onlinepubs/009695399/functions/posix_spawn.html

All choices are not thread-safe.

> By the way, I wonder if Boost.Test likes a test case which closes all 
> file descriptors? It would be nice if this could be added as a test case.

You could dup std descriptors to some upper fds before the test and dup
them back after the test.