You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Qian Zhang <zh...@gmail.com> on 2018/09/06 01:25:33 UTC
Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/
-----------------------------------------------------------
Review request for mesos, Gilbert Song and James Peach.
Bugs: MESOS-9152
https://issues.apache.org/jira/browse/MESOS-9152
Repository: mesos
Description
-------
Closed all file descriptors except `whitelist_fds` in posix/subprocess.
Diffs
-----
3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
Diff: https://reviews.apache.org/r/68644/diff/1/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Qian Zhang <zh...@gmail.com>.
> On Sept. 7, 2018, 2:18 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 195 (patched)
> > <https://reviews.apache.org/r/68644/diff/1/?file=2082817#file2082817line195>
> >
> > We need to be careful here. We are in an async-signal-safe context but hashmap and list allocate memory.
> >
> > Actually iterating over the directory is difficult to do in an async-signal-safe context. You can open the directory in the parent and do the iteration in the child, but I don't think there's any guarantee that the readdir in the child is safe (though AFAIK in glibc it would work).
Good catch!
Yeah, we could do `os::lsof()` in the parent (e.g., right before we fork the child: https://github.com/apache/mesos/blob/1.7.0/3rdparty/libprocess/src/posix/subprocess.hpp#L261) , but what if the parent opens a new fd after `os::lsof()` is called but before the child is forked? In such case, the new fd will be leaked to the child.
And it seems `opendir`, `readdir` and `closedir` are not async-signal-safe according to http://man7.org/linux/man-pages/man7/signal-safety.7.html
Maybe we should do something like this: https://github.com/python/cpython/blob/master/Modules/_posixsubprocess.c#L257:L303 , but I see they call `getdents64` which seems not async-signal-safe too.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review208417
-----------------------------------------------------------
On Sept. 6, 2018, 9:25 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2018, 9:25 a.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by James Peach <jp...@apache.org>.
> On Sept. 6, 2018, 6:18 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 195 (patched)
> > <https://reviews.apache.org/r/68644/diff/1/?file=2082817#file2082817line195>
> >
> > We need to be careful here. We are in an async-signal-safe context but hashmap and list allocate memory.
> >
> > Actually iterating over the directory is difficult to do in an async-signal-safe context. You can open the directory in the parent and do the iteration in the child, but I don't think there's any guarantee that the readdir in the child is safe (though AFAIK in glibc it would work).
>
> Qian Zhang wrote:
> Good catch!
>
> Yeah, we could do `os::lsof()` in the parent (e.g., right before we fork the child: https://github.com/apache/mesos/blob/1.7.0/3rdparty/libprocess/src/posix/subprocess.hpp#L261) , but what if the parent opens a new fd after `os::lsof()` is called but before the child is forked? In such case, the new fd will be leaked to the child.
>
> And it seems `opendir`, `readdir` and `closedir` are not async-signal-safe according to http://man7.org/linux/man-pages/man7/signal-safety.7.html
>
> Maybe we should do something like this: https://github.com/python/cpython/blob/master/Modules/_posixsubprocess.c#L257:L303 , but I see they call `getdents64` which seems not async-signal-safe too.
> Yeah, we could do os::lsof() in the parent (e.g., right before we fork the child: https://github.com/apache/mesos/blob/1.7.0/3rdparty/libprocess/src/posix/subprocess.hpp#L261) , but what if the parent opens a new fd after os::lsof() is called but before the child is forked? In such case, the new fd will be leaked to the child.
Yes, I agree that we should try to avoid this race.
> And it seems opendir, readdir and closedir are not async-signal-safe according to
> http://man7.org/linux/man-pages/man7/signal-safety.7.html
Yup. I believe that this is OK in practice on Linux; at least I see this approach being used in real code.
> Maybe we should do something like this: https://github.com/python/cpython/blob/master/Modules/_posixsubprocess.c#L257:L303 , but I see they call getdents64 which seems not async-signal-safe too.
The two approaches that I can find in common use for Linux are to scan the `fd` directory or to just close everything up to `getdtablesize`. The former is a bit less portable, and the latter probably has a small performance cost. The cpython code is probably the best implementation of the scanning approach that I've seen. I don't know of any way to implement the scanning approach that is *strictly* async-signal-safe.
A slightly different approach would be to exec a helper tool that does the closing. This would have a performance drawback (cost of linking libmesos at startup), and possibly compatibility issues.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review208417
-----------------------------------------------------------
On Sept. 6, 2018, 1:25 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2018, 1:25 a.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Qian Zhang <zh...@gmail.com>.
> On Sept. 7, 2018, 2:18 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 195 (patched)
> > <https://reviews.apache.org/r/68644/diff/1/?file=2082817#file2082817line195>
> >
> > We need to be careful here. We are in an async-signal-safe context but hashmap and list allocate memory.
> >
> > Actually iterating over the directory is difficult to do in an async-signal-safe context. You can open the directory in the parent and do the iteration in the child, but I don't think there's any guarantee that the readdir in the child is safe (though AFAIK in glibc it would work).
>
> Qian Zhang wrote:
> Good catch!
>
> Yeah, we could do `os::lsof()` in the parent (e.g., right before we fork the child: https://github.com/apache/mesos/blob/1.7.0/3rdparty/libprocess/src/posix/subprocess.hpp#L261) , but what if the parent opens a new fd after `os::lsof()` is called but before the child is forked? In such case, the new fd will be leaked to the child.
>
> And it seems `opendir`, `readdir` and `closedir` are not async-signal-safe according to http://man7.org/linux/man-pages/man7/signal-safety.7.html
>
> Maybe we should do something like this: https://github.com/python/cpython/blob/master/Modules/_posixsubprocess.c#L257:L303 , but I see they call `getdents64` which seems not async-signal-safe too.
>
> James Peach wrote:
> > Yeah, we could do os::lsof() in the parent (e.g., right before we fork the child: https://github.com/apache/mesos/blob/1.7.0/3rdparty/libprocess/src/posix/subprocess.hpp#L261) , but what if the parent opens a new fd after os::lsof() is called but before the child is forked? In such case, the new fd will be leaked to the child.
>
> Yes, I agree that we should try to avoid this race.
>
> > And it seems opendir, readdir and closedir are not async-signal-safe according to
> > http://man7.org/linux/man-pages/man7/signal-safety.7.html
>
> Yup. I believe that this is OK in practice on Linux; at least I see this approach being used in real code.
>
> > Maybe we should do something like this: https://github.com/python/cpython/blob/master/Modules/_posixsubprocess.c#L257:L303 , but I see they call getdents64 which seems not async-signal-safe too.
>
> The two approaches that I can find in common use for Linux are to scan the `fd` directory or to just close everything up to `getdtablesize`. The former is a bit less portable, and the latter probably has a small performance cost. The cpython code is probably the best implementation of the scanning approach that I've seen. I don't know of any way to implement the scanning approach that is *strictly* async-signal-safe.
>
> A slightly different approach would be to exec a helper tool that does the closing. This would have a performance drawback (cost of linking libmesos at startup), and possibly compatibility issues.
I updated the patch by following what the cpython code did. But I only did it for Linux not for macOS and FreeBSD yet, Apple has decided to deprecate all syscall functions, so we have no async-signal-safe way to get entries from the `/dev/fd` dir on macOS.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review208417
-----------------------------------------------------------
On Oct. 11, 2018, 10:03 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2018, 10:03 p.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review208417
-----------------------------------------------------------
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 195 (patched)
<https://reviews.apache.org/r/68644/#comment292321>
We need to be careful here. We are in an async-signal-safe context but hashmap and list allocate memory.
Actually iterating over the directory is difficult to do in an async-signal-safe context. You can open the directory in the parent and do the iteration in the child, but I don't think there's any guarantee that the readdir in the child is safe (though AFAIK in glibc it would work).
- James Peach
On Sept. 6, 2018, 1:25 a.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Sept. 6, 2018, 1:25 a.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Qian Zhang <zh...@gmail.com>.
> On Oct. 13, 2018, 1:40 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 213 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line213>
> >
> > Since you are planning a different code path for macOS, maybe hoist this out into a static support function in preparation?
Did you mean we should put those codes into a static function like `static int convertStringToInt(const char *name)`?
> On Oct. 13, 2018, 1:40 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 255 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line255>
> >
> > You can just use `std::find()` here.
That's what I was thinking. But it seems `std::find()` may allocate memory (search `allocate memory` in https://en.cppreference.com/w/cpp/algorithm/find )?
> On Oct. 13, 2018, 1:40 a.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 275 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line275>
> >
> > Unfortunately, the `Try` here is not async-signal-safe. However, that is already used by `UNSET_CLOEXEC`, so I think we can just leave a TODO here.
> >
> > Can you file a JIRA to add something like `signal_save::uncloexec()`?
I see we also use `Try` in another place in `childMain`, e.g., `Try<Nothing> callback = hook();`, so that one needs to be changed too?
And I'd like to call `fcntl` here directly to unset the `close-on-exec` flag.
- Qian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review209489
-----------------------------------------------------------
On Oct. 11, 2018, 10:03 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2018, 10:03 p.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by James Peach <jp...@apache.org>.
> On Oct. 12, 2018, 5:40 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 213 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line213>
> >
> > Since you are planning a different code path for macOS, maybe hoist this out into a static support function in preparation?
>
> Qian Zhang wrote:
> Did you mean we should put those codes into a static function like `static int convertStringToInt(const char *name)`?
Yes, exactly.
> On Oct. 12, 2018, 5:40 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 255 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line255>
> >
> > You can just use `std::find()` here.
>
> Qian Zhang wrote:
> That's what I was thinking. But it seems `std::find()` may allocate memory (search `allocate memory` in https://en.cppreference.com/w/cpp/algorithm/find )?
Oh, I guess that it's not guaranteed to not allocate. Let's drop this then.
> On Oct. 12, 2018, 5:40 p.m., James Peach wrote:
> > 3rdparty/libprocess/src/posix/subprocess.hpp
> > Lines 275 (patched)
> > <https://reviews.apache.org/r/68644/diff/2/?file=2096378#file2096378line275>
> >
> > Unfortunately, the `Try` here is not async-signal-safe. However, that is already used by `UNSET_CLOEXEC`, so I think we can just leave a TODO here.
> >
> > Can you file a JIRA to add something like `signal_save::uncloexec()`?
>
> Qian Zhang wrote:
> I see we also use `Try` in another place in `childMain`, e.g., `Try<Nothing> callback = hook();`, so that one needs to be changed too?
>
> And I'd like to call `fcntl` here directly to unset the `close-on-exec` flag.
> I see we also use Try in another place in childMain, e.g., Try<Nothing> callback = hook();, so that one needs to be changed too?
Yes, in principle. I don't think we need to address that here though.
> And I'd like to call fcntl here directly to unset the close-on-exec flag.
That seems fine to me. Previously, we put helpers in the `signal_safe` namespace, but having a local helper for this case seems OK too.
- James
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review209489
-----------------------------------------------------------
On Oct. 14, 2018, 2:05 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2018, 2:05 p.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review209489
-----------------------------------------------------------
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 213 (patched)
<https://reviews.apache.org/r/68644/#comment293959>
Since you are planning a different code path for macOS, maybe hoist this out into a static support function in preparation?
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 214 (patched)
<https://reviews.apache.org/r/68644/#comment293955>
How about we rephrase this as:
```
Close all file descriptors that are not explicitly whitelisted to avoid ...
```
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 233 (patched)
<https://reviews.apache.org/r/68644/#comment293958>
We should explicitly:
```C
#include <sys/syscall.h>
```
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 244 (patched)
<https://reviews.apache.org/r/68644/#comment293957>
I think our guidelines would say this should use `reinterpret_cast`?
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 255 (patched)
<https://reviews.apache.org/r/68644/#comment293956>
You can just use `std::find()` here.
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 275 (patched)
<https://reviews.apache.org/r/68644/#comment293954>
Unfortunately, the `Try` here is not async-signal-safe. However, that is already used by `UNSET_CLOEXEC`, so I think we can just leave a TODO here.
Can you file a JIRA to add something like `signal_save::uncloexec()`?
- James Peach
On Oct. 11, 2018, 2:03 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Oct. 11, 2018, 2:03 p.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9152
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Gilbert Song <so...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review210408
-----------------------------------------------------------
Ship it!
Ship It!
- Gilbert Song
On Oct. 16, 2018, 11:43 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Oct. 16, 2018, 11:43 p.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/4/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/
-----------------------------------------------------------
(Updated Oct. 17, 2018, 2:43 p.m.)
Review request for mesos, Gilbert Song and James Peach.
Changes
-------
Addressed review comments.
Bugs: MESOS-9152 and MESOS-9164
https://issues.apache.org/jira/browse/MESOS-9152
https://issues.apache.org/jira/browse/MESOS-9164
Repository: mesos
Description
-------
Closed all file descriptors except `whitelist_fds` in posix/subprocess.
Diffs (updated)
-----
3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
Diff: https://reviews.apache.org/r/68644/diff/4/
Changes: https://reviews.apache.org/r/68644/diff/3-4/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review209636
-----------------------------------------------------------
Ship it!
- James Peach
On Oct. 14, 2018, 2:05 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2018, 2:05 p.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by James Peach <jp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/#review209635
-----------------------------------------------------------
3rdparty/libprocess/src/posix/subprocess.hpp
Lines 19 (patched)
<https://reviews.apache.org/r/68644/#comment294175>
Should this be inside the `__linux__` guard?
- James Peach
On Oct. 14, 2018, 2:05 p.m., Qian Zhang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68644/
> -----------------------------------------------------------
>
> (Updated Oct. 14, 2018, 2:05 p.m.)
>
>
> Review request for mesos, Gilbert Song and James Peach.
>
>
> Bugs: MESOS-9152 and MESOS-9164
> https://issues.apache.org/jira/browse/MESOS-9152
> https://issues.apache.org/jira/browse/MESOS-9164
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Closed all file descriptors except `whitelist_fds` in posix/subprocess.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
> 3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
>
>
> Diff: https://reviews.apache.org/r/68644/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Qian Zhang
>
>
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/
-----------------------------------------------------------
(Updated Oct. 14, 2018, 10:05 p.m.)
Review request for mesos, Gilbert Song and James Peach.
Bugs: MESOS-9152 and MESOS-9164
https://issues.apache.org/jira/browse/MESOS-9152
https://issues.apache.org/jira/browse/MESOS-9164
Repository: mesos
Description
-------
Closed all file descriptors except `whitelist_fds` in posix/subprocess.
Diffs
-----
3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
Diff: https://reviews.apache.org/r/68644/diff/3/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/
-----------------------------------------------------------
(Updated Oct. 14, 2018, 9:52 p.m.)
Review request for mesos, Gilbert Song and James Peach.
Changes
-------
Addressed review comments.
Bugs: MESOS-9152
https://issues.apache.org/jira/browse/MESOS-9152
Repository: mesos
Description
-------
Closed all file descriptors except `whitelist_fds` in posix/subprocess.
Diffs (updated)
-----
3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
Diff: https://reviews.apache.org/r/68644/diff/3/
Changes: https://reviews.apache.org/r/68644/diff/2-3/
Testing
-------
Thanks,
Qian Zhang
Re: Review Request 68644: Closed all file descriptors except
`whitelist_fds` in posix/subprocess.
Posted by Qian Zhang <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/68644/
-----------------------------------------------------------
(Updated Oct. 11, 2018, 10:03 p.m.)
Review request for mesos, Gilbert Song and James Peach.
Changes
-------
Addressed review comments.
Bugs: MESOS-9152
https://issues.apache.org/jira/browse/MESOS-9152
Repository: mesos
Description
-------
Closed all file descriptors except `whitelist_fds` in posix/subprocess.
Diffs (updated)
-----
3rdparty/libprocess/src/posix/subprocess.hpp 007058b61fdcd4716aa793516c842c3cef8c0a29
3rdparty/libprocess/src/subprocess.cpp c0640de2dc4278b884282dfaad98c49c3b067a5b
Diff: https://reviews.apache.org/r/68644/diff/2/
Changes: https://reviews.apache.org/r/68644/diff/1-2/
Testing
-------
Thanks,
Qian Zhang