Re: [boost] [Boost-announce] [Boost-users] Subject: Formal Review of Proposed Boost.Process library starts tomorrow

Subject: Re: [boost] [Boost-announce] [Boost-users] Subject: Formal Review of Proposed Boost.Process library starts tomorrow
From: Boris Schaeling (boris_at_[hidden])
Date: 2011-02-16 19:32:16

On Wed, 16 Feb 2011 11:28:41 +0100, Brendon Costa  
<brendon.j.costa_at_[hidden]> wrote:

Hi Brendon,

thanks for your review! I'll quickly go through it.

> [...]1) Requiring #ifdef's to handle the standard case of the exit status
> is nasty. I dont want to HAVE to use #ifdef statements for the most
> common usage. A better abstraction should be made as has been
> mentioned in previous reviews.

I think that's fine - as long as we can agree on what the most common  
usage is. :) The question about the exit code and the POSIX macros is: Can  
Boost.Process do anything by default most POSIX developers would be happy  
with if WIFEXITED returns false?

> [...]3) There is code to send common termination signals to child
> processes, I also think a simple abstraction could be provided for
> child processes to use to register handlers for the normal termination
> events (On POSIX Handling sigterm at leastfor example).

I think a signal handler would be a nice contribution. I'll send another  
email at the end of the review though to explain the rationale and what  
conclusions this review helped me to draw.

> [...]I think this is possible by creating an anonymous pipe before the  
> fork
> that is used to communicate errors from the child to the parent that
> may occur before the exec has been called. The FD of this pipe in the
> child should be marked to close on exec and thus will close on
> successful exec and if exec fails can be used to send that error back
> to the parent as well. The parent should then read these errors from
> the pipe and throw an exception. Otherwise you rely on writing to
> stderr in the child (bad, what if i dont want these errors printed to
> stderr) and exiting with a status and this looks like the exec'ed
> process failed, not the process of creating an execed child failed. It
> does mean that the parent will need to synchronise with the exec of
> the client possibly waiting for a short period of time (waiting for
> the error pipe to close before moving on).

Thanks for this new idea! That's interesting as it would allow the parent  
process to detect if something goes wrong (and what). Noted!

> 6) I didnt look very closely and do testing under linux, but it looks
> like in windows we hold a handle reference to the process to prevent
> it being cleaned up so we can get the exit status of the process. This

Yes, exactly.

> [...]10) I didnt get a good feel for what the error handling strategies  
> are
> for this library from the docs. What errors can be expected where? Not

Good point, also noted.

Thanks,
Boris

> [...]