On Wed, 16 Feb 2011 11:28:41 +0100, Brendon Costa <brendon.j.costa@gmail.com> 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
[...]