steve7411
commented
7 years ago I forgot to mention, but there were also a number of potential error cases in the C/posix code that could lead to the methods returning before they'd freed kernel resources.
I'm not sure if you guys will be interested in this change since it turned out to be more of a rewrite than I expected:
With the previous Mac/Linux thread, if a user had anything that wrote to either stdout or stderr in their bash profile or bashrc, the request would fail. I initially tried running bash with --noprofile and --norc and passing the PATH environment directly to the command, but I couldn't quite get that to work. So I changed it so that just having something written to stderr isn't automatically a failure, but is only logged as a warning so long as the last line written to stdout appears to be what we expect from the which command and the ping command has the expected "time=" and "ms\n" tokens.
Another problem is that with pselect, you get notified when the stdout/stderr pipes are ready to be read, but that doesn't mean the process is done outputting to those pipes. So if the user had some commands that output to stdout in their profile or bashrc, in most cases you will only get some of that output after a single pselect and read (timing dependent). I initially wanted to correct for this by simply writing a polling loop for the thread and waiting until it stopped with the existing posix calls, but it turns out that that's quite a bit of code, and Unreal already has this implemented in their FPlatformProcess classes. The MacLinuxPingThread::RunBashCommand method that I added is essentially copy and pasted from the Linux implementation of FPlatformProcess::ExecProcess, and the only reason I didn't call that method directly is to support the existing timeout functionality, which Unreal does not seem to support.
I also removed the call to which ping, because which uses the same algorithm as bash to look up the executable on the PATH, so I'm not sure which edge cases that addressed. I did port the method over, and I simply commented out the calls to it, so it should be trivial to put back in.