mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joseph Wu <jos...@mesosphere.io>
Subject Re: Review Request 56689: Windows: Prevented crash if sandbox `stdout` file already exists.
Date Thu, 09 Mar 2017 20:27:26 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56689/#review168509
-----------------------------------------------------------


Ship it!




Ship It!

- Joseph Wu


On Feb. 14, 2017, 3:40 p.m., Alex Clemmer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56689/
> -----------------------------------------------------------
> 
> (Updated Feb. 14, 2017, 3:40 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer, Daniel Pravat, and Joseph Wu.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Subprocess currently provides a cross-platform API that allows the child
> process to redirect `stdin`, `stdout`, and `stderr`, with support for
> file paths, file descriptors, pipes, or Windows file `HANDLE`s.
> 
> Currently, in the case of file paths, the Windows code will use
> `CreateFile` to open the file, with the semantics that it will error out
> if the file already exists (because of the `CREATE_NEW` flag), and
> explicitly specifying that it is ok to have multiple processes writing
> to the file (because of the `FILE_SHARE_WRITE` flag).
> 
> The former of these is undesirable; libprocess should be able to proceed
> regardless of whether the file already exists. But this also causes bugs
> to the downstream, namely Mesos, in which a Fetcher might create the
> sandbox `stdout` folder, and then subsequently crash when it tries to
> open the `stdout` folder in the executor.
> 
> In this commit, we change the `CREATE_NEW` flag to `OPEN_ALWAYS`, which
> has the semantics that we will create the file if it does not exist, and
> open it if it does.
> 
> Additionally, since the documentation does not specify whether
> `FILE_SHARE_WRITE` also allows other processes to have read access, we
> additionally specify the flag `FILE_SHARE_READ` just to be sure we make
> no assumptions about who is able to read and write to these files. See
> comments for additional context on this point.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/src/subprocess_windows.cpp 1b5ae5e3ee3c08d05afa1bc533542f42fe26eed0

> 
> 
> Diff: https://reviews.apache.org/r/56689/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alex Clemmer
> 
>


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