- How much effort did you put into your evaluation? A glance? A quick reading? In-depth study?
This review is based on a read of the documentation, reading select parts of the code, as well as a prior evaluation of an earlier version of the library. I didn't actually write or compile any code.
- What is your evaluation of the design?
1. The stream behavior interface adds an unnecessary additional layer of abstraction, solely to allow reuse of the context instance, which seems unlikely to be useful, and certainly not worth the added complexity cost. I suggest the following simpler interface: Individual stream_ids are mapped to single (as opposed to pairs of) file descriptors (e.g. boost::iostreams::file_descriptor). Boost process takes care of setting up these mappings. A convenience interface for inherit could still be provided. If some of these file descriptors are pipes, the user code should have maintained a reference to the appropriate end, on its own. Related to this, the pipe creation facility should be written as a general facility independent of Boost.Process (even though it might well officially be contained in it), as should the null file facility. It also should not be tied to C++ iostreams; instead, users could make use of the existing facilities in Boost.Iostreams to obtain a C++ iostream interface to a file descriptor. 2. On POSIX, the asynchronous wait interface does not provide notification of child processes being suspended or continued. 3. Unicode support for Windows (for executable names/paths, command-line arguments, and environment variables) is needed. 4. A simple cross-platform interface to exit status might be useful, e.g. boost::optional<int> normal_exit_status(), where on Windows there is always a value, and on POSIX the value is present only if the process exited normally. 5. On Windows, the extension interface may be too limited. 6. Some simple convenience functions for common tasks such as running a process, waiting (either synchronously or asynchronously) for it to exit, and receiving its STDOUT output as a string, might be useful.
- What is your evaluation of the implementation?
On POSIX, the asynchronous wait interface is quite inefficient, as it requires an extra thread per child process. Instead, waiting based on SIGCHLD should be used. Interoperability with other common C/C++ libraries that deal with process creation, such as glib, when possible would be desirable. On POSIX, errors that occur in the child process after fork() but before execve() should be distinguishable from an exit status that occurs after execve().
- What is your evaluation of the documentation?
Many operations in the library, while to some extent platform-independent, nonetheless have platform-specific behavior that should be documented. For instance, it should be documented that process::terminate calls TerminateProcess on Windows (because then users can look up the specific behavior of TerminateProcess) and ignores the force argument, while on POSIX it sends a SIGTERM signal (or SIGKILL signal if force is true). The note that accessing the process object after calling terminate may be dangerous is incorrect, I believe. On POSIX, the PID is valid until it is returned by a call to wait/waitpid. Thus, on POSIX, if the process is not a child process, the PID may become invalid at any time, before or after the call to terminate. On Windows, the PID is valid until the last handle is closed, and the process class specifically keeps a handle open for this reason, and therefore regardless of whether or not it is a child process, the PID will remain valid. Additionally, on POSIX, it should be documented precisely when the context::setup function is invoked, in relation to other process setup operations, such as changing the working directory, and setting up file descriptors.
- What is your evaluation of the potential usefulness of the library?
If only the synchronous waiting is to be used, or only a small number of processes are to be created, the library is quite adequate for use on POSIX. The support for mapping arbitrary file descriptors, rather than just 0, 1, and 2, makes it more powerful than some other libraries, such as glib. On Windows, it is of limited use as is, due to lack of Unicode support in the various strings, though that could probably be fixed relatively easily.
- Are you knowledgeable about the problem domain?
I am fairly knowledgeable about the complexities of process creation and signal handling on POSIX platforms, having written several simple C++ interfaces for process creation (on top of the POSIX system API) for use in my own software. I also have a reasonable understanding of process creation in Windows (and I believe Windows is fairly straightforward).
And finally, every review should answer this question:
- Do you think the library should be accepted as a Boost library?
The library should not be accepted as is. The lack of efficient asynchronous waiting support, and the overly complicated stream behavior interface, are showstoppers for me.