You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chi Zhang <ch...@gmail.com> on 2015/09/24 02:21:46 UTC
Review Request 38699: stout: use os::close everywhere.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38699/
-----------------------------------------------------------
Review request for mesos and Vinod Kone.
Bugs: mesos-2768
https://issues.apache.org/jira/browse/mesos-2768
Repository: mesos
Description
-------
stout: use os::close everywhere.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 7eb51e8771e95f57548fc35753e75c6d56cd97cd
Diff: https://reviews.apache.org/r/38699/diff/
Testing
-------
Thanks,
Chi Zhang
Re: Review Request 38699: stout: use os::close everywhere.
Posted by Cong Wang <cw...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38699/#review100344
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp (line 266)
<https://reviews.apache.org/r/38699/#comment157536>
I must miss something too obvious, but... doesn't os::close() return a Try<Nothing>? How could that be compared with -1?
- Cong Wang
On Sept. 24, 2015, 12:21 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38699/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2015, 12:21 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: mesos-2768
> https://issues.apache.org/jira/browse/mesos-2768
>
>
> Repository: mesos
>
>
> Description
> -------
>
> stout: use os::close everywhere.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 7eb51e8771e95f57548fc35753e75c6d56cd97cd
>
> Diff: https://reviews.apache.org/r/38699/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Chi Zhang
>
>
Re: Review Request 38699: stout: use os::close everywhere.
Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38699/#review100329
-----------------------------------------------------------
3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp (line 266)
<https://reviews.apache.org/r/38699/#comment157513>
Is it possible for the shared memory deleter to execute in the forked context? Or are we guaranteed that the last reference is in the parent? If not, this change seems problematic since close is not async signal safe (Try uses new which uses malloc). It looks like ABORT with std::string is also problematic in the error case.
- Ben Mahler
On Sept. 24, 2015, 12:21 a.m., Chi Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38699/
> -----------------------------------------------------------
>
> (Updated Sept. 24, 2015, 12:21 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: mesos-2768
> https://issues.apache.org/jira/browse/mesos-2768
>
>
> Repository: mesos
>
>
> Description
> -------
>
> stout: use os::close everywhere.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/include/stout/os/posix/fork.hpp 7eb51e8771e95f57548fc35753e75c6d56cd97cd
>
> Diff: https://reviews.apache.org/r/38699/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Chi Zhang
>
>