mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Akash Gupta <akash-gu...@hotmail.com>
Subject Re: Review Request 65574: Windows: Fixed handle inheritance in `create_process` wrapper.
Date Wed, 21 Feb 2018 00:01:23 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message