----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65574/#review197823 -----------------------------------------------------------
3rdparty/stout/include/stout/os/windows/shell.hpp Lines 275 (patched) <https://reviews.apache.org/r/65574/#comment278116> It seems like you can have a race condition here if you send the same handles to two different processes at the same time. If the second process finishes executing this and then the first process exits and sets the handles to uninheritable, then you would set uninheritable handles to the child process. If this function is supposed to have this behavior, maybe add a comment explaining that this function mutates the handles, so the caller needs to make sure to synchronize these calls. 3rdparty/stout/include/stout/os/windows/shell.hpp Line 294 (original), 301 (patched) <https://reviews.apache.org/r/65574/#comment278113> Hm, I remember we discussed something like `CreateProcessW` does process initialization async. Do we need to wait for the process to be initialized? - Akash Gupta On Feb. 20, 2018, 7:39 p.m., Andrew Schwartzmeyer wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/65574/ > ----------------------------------------------------------- > > (Updated Feb. 20, 2018, 7:39 p.m.) > > > Review request for mesos, Akash Gupta, Jeff Coffler, Jie Yu, and Joseph Wu. > > > Bugs: MESOS-6838 and MESOS-8512 > https://issues.apache.org/jira/browse/MESOS-6838 > https://issues.apache.org/jira/browse/MESOS-8512 > > > Repository: mesos > > > Description > ------- > > The primary change in this patch is that `create_process` now enables > inheritance for the `pipe` handles passed before starting the child > process. This is required, otherwise the child process will behave > incorrectly (for example, it will write to `stdout` but that will go > nowhere, as the redirection silently failed). After the process is > created, inheritance is disabled to prevent further calls to > `create_process` from inheriting the wrong handles. > > The `std::tuple<os::WindowsFD, ...>` type was changed to a > `std::array<os::WindowsFD, 3>` as it is significantly easier to work > with (it supports iteration). > > > Diffs > ----- > > 3rdparty/stout/include/stout/os/windows/shell.hpp > 542039c31f94eda1af121335b12edf9b83725ae5 > > > Diff: https://reviews.apache.org/r/65574/diff/1/ > > > Testing > ------- > > > Thanks, > > Andrew Schwartzmeyer > >
