The _spawnvp in src/std/process.d is broken. First, if the child process (say it has pid B) fails to execvp, it's a bad idea to then throw an Exception. The entire point of spawnvp in general is to hide the fact that the current process A was forked off. But now the Exception propagates through process B, so certainly the caller will notice that something is off (the caller is suddenly inside another process than he started out in). Later on, the waitpid result is not checked. It is possible for waitpid to return (-1). In that case, errno contains the error code and "status" contains garbage, which is then compared against. Also, all Posix system calls can return (-1) and errno = EINTR (see <http://www.jwz.org/doc/worse-is-better.html>, search for "PC loser-ing") to indicate that while the user process asked for action S to be performed, really it should be checking and doing some other action T before. So for the latter there really should be some global delegate that is called on EINTR which decides whether to do anything about it, possibly terminating the loop (or not, it depends). This is not specific to process.d but all functions that do system calls should call this. Even std.stdio.File functions should do this. Also, it throws an Exception (literally that) using strerror_r to build it instead of just using ErrnoException. Why?
Created attachment 1454 [details] Tests whether spawnvp magically forks processes for the caller Tests whether spawnvp magically switches processes. Try with version(Posix) implementation of spawnvp.
Created attachment 1455 [details] Patch to make _spawnvp less bad checks waitpid() return value, does not magically put the caller into a new process. Does not properly handle EINTR.
See http://www.gnu.org/software/libc/manual/html_node/Interrupted-Primitives.html
Also, both the OSX and the Posix version of browse in the same file are broken in the same way...
spawnvp seems only to exist for windows now. But I did not check if this has been the case in 2014 too. Tried to adapt the test for "browse": ``` import std.process; import core.thread; int main() { auto pidBefore = getpid(); try { browse("DOESNOTEXIST"); assert(false); } catch(Exception e) { auto pidAfterwards = getpid(); assert(pidBefore == pidAfterwards); // make sure we are still in the same process } return 0; } ``` This produces an Assertion failure (POSIX). And the process seems still to be running after the main process has stopped.
THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/phobos/issues/10099 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB