You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Till Toenshoff <to...@me.com> on 2014/03/16 12:05:31 UTC
Review Request 19259: Added ability to process::subprocess to run function
within child context
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
Bugs: MESOS-1102
https://issues.apache.org/jira/browse/MESOS-1102
Repository: mesos-git
Description
-------
Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
Diffs
-----
3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
Diff: https://reviews.apache.org/r/19259/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
> On March 18, 2014, 6:39 p.m., Ian Downes wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, line 165
> > <https://reviews.apache.org/r/19259/diff/2/?file=520576#file520576line165>
> >
> > what about either exec command *or* call child function (which can choose to exec{l,le,...})
I like this idea a lot as it also gives the caller full control of the actual invocation (e.g. via /bin/sh or not via using execv(e)).
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37588
-----------------------------------------------------------
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On March 18, 2014, 6:39 p.m., Ian Downes wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, line 165
> > <https://reviews.apache.org/r/19259/diff/2/?file=520576#file520576line165>
> >
> > what about either exec command *or* call child function (which can choose to exec{l,le,...})
>
> Till Toenshoff wrote:
> I like this idea a lot as it also gives the caller full control of the actual invocation (e.g. via /bin/sh or not via using execv(e)).
I think there are use cases where it would be nice to just do some 'setup' after fork but before exec, so I'm not convinced we should do an *or* because that means we'll be asking everyone to do their own 'exec*(...)' which is kind of a pain. At least for now, let's go with an async-signal-safe 'setup' and then exec'ing the 'command' via /bin/sh.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37588
-----------------------------------------------------------
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Ian Downes <ia...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37588
-----------------------------------------------------------
3rdparty/libprocess/src/subprocess.cpp
<https://reviews.apache.org/r/19259/#comment69184>
what about either exec command *or* call child function (which can choose to exec{l,le,...})
- Ian Downes
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Ian Downes <ia...@gmail.com>.
> On March 17, 2014, 4:57 p.m., Niklas Nielsen wrote:
> > 3rdparty/libprocess/src/tests/subprocess_tests.cpp, line 345
> > <https://reviews.apache.org/r/19259/diff/2/?file=520577#file520577line345>
> >
> > How about using mktemp instead? Also, do we clean up after the test?
Yes, this is better done in SetUpTestCase() and TearDownTestCase(), if possible.
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37401
-----------------------------------------------------------
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37401
-----------------------------------------------------------
3rdparty/libprocess/src/tests/subprocess_tests.cpp
<https://reviews.apache.org/r/19259/#comment68913>
I think we usually wrap before ':' and keep the member variables on separate lines. I might be wrong of course.
3rdparty/libprocess/src/tests/subprocess_tests.cpp
<https://reviews.apache.org/r/19259/#comment68912>
2 indents.
3rdparty/libprocess/src/tests/subprocess_tests.cpp
<https://reviews.apache.org/r/19259/#comment68911>
Add brackets :)
3rdparty/libprocess/src/tests/subprocess_tests.cpp
<https://reviews.apache.org/r/19259/#comment68914>
How about using mktemp instead? Also, do we clean up after the test?
- Niklas Nielsen
On March 16, 2014, 4:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 4:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
> On April 3, 2014, 7:45 p.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, lines 141-144
> > <https://reviews.apache.org/r/19259/diff/3/?file=548212#file548212line141>
> >
> > Can we comment that we're doing this before we fork because it might not be async-signal safe?
> >
> > Also, how about we simplify this back to as it was before and just do:
> >
> > internal::Envp envp(environment.get(map<string, string>()));
I entirely missed the Option::get default override - lovely!
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review39464
-----------------------------------------------------------
On April 3, 2014, 10:46 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 10:46 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review39464
-----------------------------------------------------------
I think we can simplify by just having the forked child use '_exit' if 'setup' fails. Check out the comments below. Great!
3rdparty/libprocess/src/subprocess.cpp
<https://reviews.apache.org/r/19259/#comment71855>
Can we comment that we're doing this before we fork because it might not be async-signal safe?
Also, how about we simplify this back to as it was before and just do:
internal::Envp envp(environment.get(map<string, string>()));
3rdparty/libprocess/src/subprocess.cpp
<https://reviews.apache.org/r/19259/#comment71853>
IIUC, an '_exit' should be sufficient for non-zero return codes, should clean this up too:
if (setup.isSome()) {
int status = setup.get()();
if (status != 0) {
_exit(status);
}
}
- Benjamin Hindman
On April 3, 2014, 7:23 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 7:23 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review39493
-----------------------------------------------------------
Ship it!
Awesome Till, I'll get this committed now.
- Benjamin Hindman
On April 3, 2014, 10:46 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 10:46 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review39486
-----------------------------------------------------------
Ship it!
Ship It!
- Dominic Hamon
On April 3, 2014, 3:46 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 3:46 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/
-----------------------------------------------------------
(Updated April 3, 2014, 10:46 p.m.)
Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
Bugs: MESOS-1102
https://issues.apache.org/jira/browse/MESOS-1102
Repository: mesos-git
Description
-------
Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
Diffs (updated)
-----
3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
3rdparty/libprocess/src/subprocess.cpp e09c808
3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
Diff: https://reviews.apache.org/r/19259/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/
-----------------------------------------------------------
(Updated April 3, 2014, 10:44 p.m.)
Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
Changes
-------
Addressed all comments. Please review again.
Bugs: MESOS-1102
https://issues.apache.org/jira/browse/MESOS-1102
Repository: mesos-git
Description
-------
Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
Diffs (updated)
-----
3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
3rdparty/libprocess/src/subprocess.cpp e09c808
3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
Diff: https://reviews.apache.org/r/19259/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On April 3, 2014, 7:32 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, line 141
> > <https://reviews.apache.org/r/19259/diff/3/?file=548212#file548212line141>
> >
> > Change Envp to take an Option<map> and skip the loop over the passed in map if it's None()?
I think it's fine to just do:
internal::Envp envp(environment.get(map<string, string>()));
As it captures the current semantics. We could add a TODO to optimize Envp to not generate the environment for empty maps (or better a None) , but I'm happy to just make that a TODO now and optimize later.
> On April 3, 2014, 7:32 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, lines 179-189
> > <https://reviews.apache.org/r/19259/diff/3/?file=548212#file548212line179>
> >
> > this isn't 'setup && command' as documented. If it was, we wouldn't run command if setup returned non-zero.
Yes, see my comments re: using _exit.
> On April 3, 2014, 7:32 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 79
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line79>
> >
> > It just feels a little restrictive, in that I might want to pass arguments.
Since 'subprocess' will never be passing arguments to the function taking 'void' is fine, you can bind your own arguments as his tests show.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37440
-----------------------------------------------------------
On April 3, 2014, 7:23 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 7:23 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
> On April 3, 2014, 7:32 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 79
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line79>
> >
> > It just feels a little restrictive, in that I might want to pass arguments.
>
> Benjamin Hindman wrote:
> Since 'subprocess' will never be passing arguments to the function taking 'void' is fine, you can bind your own arguments as his tests show.
Right, bind-override or bind-functor are options to get over this limitation.
> On April 3, 2014, 7:32 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, line 141
> > <https://reviews.apache.org/r/19259/diff/3/?file=548212#file548212line141>
> >
> > Change Envp to take an Option<map> and skip the loop over the passed in map if it's None()?
>
> Benjamin Hindman wrote:
> I think it's fine to just do:
>
> internal::Envp envp(environment.get(map<string, string>()));
>
> As it captures the current semantics. We could add a TODO to optimize Envp to not generate the environment for empty maps (or better a None) , but I'm happy to just make that a TODO now and optimize later.
Option::get(*default*) was new to me. Thanks for pointing this out Ben. I added that TODO with another point; which is to not use execle in the first place when Environment.isNone() as we are emulating what execl already does for us.
> On April 3, 2014, 7:32 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, lines 179-189
> > <https://reviews.apache.org/r/19259/diff/3/?file=548212#file548212line179>
> >
> > this isn't 'setup && command' as documented. If it was, we wouldn't run command if setup returned non-zero.
>
> Benjamin Hindman wrote:
> Yes, see my comments re: using _exit.
Ah, I got the initial plan entirely wrong here - never considered to just exit instead of exec'ing the command regardless.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37440
-----------------------------------------------------------
On April 3, 2014, 10:46 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 10:46 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37440
-----------------------------------------------------------
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/19259/#comment68972>
It just feels a little restrictive, in that I might want to pass arguments.
3rdparty/libprocess/src/subprocess.cpp
<https://reviews.apache.org/r/19259/#comment71851>
Change Envp to take an Option<map> and skip the loop over the passed in map if it's None()?
3rdparty/libprocess/src/subprocess.cpp
<https://reviews.apache.org/r/19259/#comment71852>
this isn't 'setup && command' as documented. If it was, we wouldn't run command if setup returned non-zero.
- Dominic Hamon
On April 3, 2014, 12:23 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 12:23 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review39467
-----------------------------------------------------------
Bad patch!
Reviews applied: [19259]
Failed command: make check
Error:
Making check in .
make[1]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot'
make[1]: Nothing to be done for `check-am'.
make[1]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot'
Making check in 3rdparty
make[1]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
make check-recursive
make[2]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
Making check in libprocess
make[3]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
Making check in 3rdparty
make[4]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make check-recursive
make[5]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
Making check in stout
make[6]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[6]: Nothing to be done for `check'.
make[6]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty/stout'
make[6]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make libgmock.la stout-tests
make[7]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make[7]: `libgmock.la' is up to date.
make[7]: `stout-tests' is up to date.
make[7]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make check-local
make[7]: Entering directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
./stout-tests
[==========] Running 118 tests from 24 test cases.
[----------] Global test environment set-up.
[----------] 3 tests from Stout
[ RUN ] Stout.Bytes
[ OK ] Stout.Bytes (0 ms)
[ RUN ] Stout.Set
[ OK ] Stout.Set (0 ms)
[ RUN ] Stout.Some
[ OK ] Stout.Some (0 ms)
[----------] 3 tests from Stout (0 ms total)
[----------] 4 tests from Cache
[ RUN ] Cache.Insert
[ OK ] Cache.Insert (0 ms)
[ RUN ] Cache.Update
[ OK ] Cache.Update (0 ms)
[ RUN ] Cache.Erase
[ OK ] Cache.Erase (0 ms)
[ RUN ] Cache.LRUEviction
[ OK ] Cache.LRUEviction (0 ms)
[----------] 4 tests from Cache (0 ms total)
[----------] 4 tests from DurationTest
[ RUN ] DurationTest.Comparison
[ OK ] DurationTest.Comparison (0 ms)
[ RUN ] DurationTest.ParseAndTry
[ OK ] DurationTest.ParseAndTry (0 ms)
[ RUN ] DurationTest.Arithmetic
[ OK ] DurationTest.Arithmetic (0 ms)
[ RUN ] DurationTest.OutputFormat
[ OK ] DurationTest.OutputFormat (0 ms)
[----------] 4 tests from DurationTest (0 ms total)
[----------] 1 test from ErrorTest
[ RUN ] ErrorTest.Test
[ OK ] ErrorTest.Test (0 ms)
[----------] 1 test from ErrorTest (0 ms total)
[----------] 12 tests from FlagsTest
[ RUN ] FlagsTest.Load
[ OK ] FlagsTest.Load (1 ms)
[ RUN ] FlagsTest.Add
[ OK ] FlagsTest.Add (0 ms)
[ RUN ] FlagsTest.Flags
[ OK ] FlagsTest.Flags (0 ms)
[ RUN ] FlagsTest.LoadFromEnvironment
[ OK ] FlagsTest.LoadFromEnvironment (0 ms)
[ RUN ] FlagsTest.LoadFromCommandLine
[ OK ] FlagsTest.LoadFromCommandLine (0 ms)
[ RUN ] FlagsTest.LoadFromCommandLineWithNonFlags
[ OK ] FlagsTest.LoadFromCommandLineWithNonFlags (0 ms)
[ RUN ] FlagsTest.Stringification
[ OK ] FlagsTest.Stringification (0 ms)
[ RUN ] FlagsTest.DuplicatesFromEnvironment
[ OK ] FlagsTest.DuplicatesFromEnvironment (0 ms)
[ RUN ] FlagsTest.DuplicatesFromCommandLine
[ OK ] FlagsTest.DuplicatesFromCommandLine (0 ms)
[ RUN ] FlagsTest.Errors
[ OK ] FlagsTest.Errors (0 ms)
[ RUN ] FlagsTest.Usage
[ OK ] FlagsTest.Usage (1 ms)
[ RUN ] FlagsTest.Duration
[ OK ] FlagsTest.Duration (0 ms)
[----------] 12 tests from FlagsTest (2 ms total)
[----------] 1 test from GzipTest
[ RUN ] GzipTest.CompressDecompressString
[ OK ] GzipTest.CompressDecompressString (114 ms)
[----------] 1 test from GzipTest (114 ms total)
[----------] 2 tests from HashMapTest
[ RUN ] HashMapTest.Insert
[ OK ] HashMapTest.Insert (0 ms)
[ RUN ] HashMapTest.Contains
[ OK ] HashMapTest.Contains (0 ms)
[----------] 2 tests from HashMapTest (0 ms total)
[----------] 2 tests from HashsetTest
[ RUN ] HashsetTest.Insert
[ OK ] HashsetTest.Insert (0 ms)
[ RUN ] HashsetTest.Union
[ OK ] HashsetTest.Union (0 ms)
[----------] 2 tests from HashsetTest (0 ms total)
[----------] 8 tests from IntervalTest
[ RUN ] IntervalTest.Interval
[ OK ] IntervalTest.Interval (0 ms)
[ RUN ] IntervalTest.EmptyInterval
[ OK ] IntervalTest.EmptyInterval (0 ms)
[ RUN ] IntervalTest.Constructor
[ OK ] IntervalTest.Constructor (0 ms)
[ RUN ] IntervalTest.Addition
[ OK ] IntervalTest.Addition (0 ms)
[ RUN ] IntervalTest.Subtraction
[ OK ] IntervalTest.Subtraction (0 ms)
[ RUN ] IntervalTest.Intersection
[ OK ] IntervalTest.Intersection (0 ms)
[ RUN ] IntervalTest.LargeInterval
[ OK ] IntervalTest.LargeInterval (0 ms)
[ RUN ] IntervalTest.IntervalIteration
[ OK ] IntervalTest.IntervalIteration (0 ms)
[----------] 8 tests from IntervalTest (0 ms total)
[----------] 8 tests from JsonTest
[ RUN ] JsonTest.DefaultValueIsNull
[ OK ] JsonTest.DefaultValueIsNull (0 ms)
[ RUN ] JsonTest.BinaryData
[ OK ] JsonTest.BinaryData (0 ms)
[ RUN ] JsonTest.NumberFormat
[ OK ] JsonTest.NumberFormat (0 ms)
[ RUN ] JsonTest.BooleanFormat
[ OK ] JsonTest.BooleanFormat (0 ms)
[ RUN ] JsonTest.BooleanAssignement
[ OK ] JsonTest.BooleanAssignement (0 ms)
[ RUN ] JsonTest.CStringAssignment
[ OK ] JsonTest.CStringAssignment (0 ms)
[ RUN ] JsonTest.NumericAssignment
[ OK ] JsonTest.NumericAssignment (0 ms)
[ RUN ] JsonTest.parse
[ OK ] JsonTest.parse (0 ms)
[----------] 8 tests from JsonTest (0 ms total)
[----------] 5 tests from LinkedHashmapTest
[ RUN ] LinkedHashmapTest.Put
[ OK ] LinkedHashmapTest.Put (0 ms)
[ RUN ] LinkedHashmapTest.Contains
[ OK ] LinkedHashmapTest.Contains (0 ms)
[ RUN ] LinkedHashmapTest.Erase
[ OK ] LinkedHashmapTest.Erase (0 ms)
[ RUN ] LinkedHashmapTest.Keys
[ OK ] LinkedHashmapTest.Keys (0 ms)
[ RUN ] LinkedHashmapTest.Values
[ OK ] LinkedHashmapTest.Values (0 ms)
[----------] 5 tests from LinkedHashmapTest (0 ms total)
[----------] 6 tests from MultimapTest/0, where TypeParam = Multimap<std::string, unsigned short>
[ RUN ] MultimapTest/0.Put
[ OK ] MultimapTest/0.Put (0 ms)
[ RUN ] MultimapTest/0.Remove
[ OK ] MultimapTest/0.Remove (0 ms)
[ RUN ] MultimapTest/0.Size
[ OK ] MultimapTest/0.Size (0 ms)
[ RUN ] MultimapTest/0.Keys
[ OK ] MultimapTest/0.Keys (0 ms)
[ RUN ] MultimapTest/0.Iterator
[ OK ] MultimapTest/0.Iterator (0 ms)
[ RUN ] MultimapTest/0.Foreach
[ OK ] MultimapTest/0.Foreach (0 ms)
[----------] 6 tests from MultimapTest/0 (0 ms total)
[----------] 6 tests from MultimapTest/1, where TypeParam = multihashmap<std::string, unsigned short>
[ RUN ] MultimapTest/1.Put
[ OK ] MultimapTest/1.Put (0 ms)
[ RUN ] MultimapTest/1.Remove
[ OK ] MultimapTest/1.Remove (0 ms)
[ RUN ] MultimapTest/1.Size
[ OK ] MultimapTest/1.Size (0 ms)
[ RUN ] MultimapTest/1.Keys
[ OK ] MultimapTest/1.Keys (0 ms)
[ RUN ] MultimapTest/1.Iterator
[ OK ] MultimapTest/1.Iterator (0 ms)
[ RUN ] MultimapTest/1.Foreach
[ OK ] MultimapTest/1.Foreach (0 ms)
[----------] 6 tests from MultimapTest/1 (0 ms total)
[----------] 2 tests from NetTest
[ RUN ] NetTest.mac
[ OK ] NetTest.mac (1 ms)
[ RUN ] NetTest.ip
[ OK ] NetTest.ip (1 ms)
[----------] 2 tests from NetTest (2 ms total)
[----------] 1 test from NoneTest
[ RUN ] NoneTest.Test
[ OK ] NoneTest.Test (0 ms)
[----------] 1 test from NoneTest (0 ms total)
[----------] 3 tests from OptionTest
[ RUN ] OptionTest.Min
[ OK ] OptionTest.Min (0 ms)
[ RUN ] OptionTest.Max
[ OK ] OptionTest.Max (0 ms)
[ RUN ] OptionTest.Comparison
[ OK ] OptionTest.Comparison (0 ms)
[----------] 3 tests from OptionTest (0 ms total)
[----------] 17 tests from OsTest
[ RUN ] OsTest.environment
[ OK ] OsTest.environment (0 ms)
[ RUN ] OsTest.rmdir
[ OK ] OsTest.rmdir (1 ms)
[ RUN ] OsTest.nonblock
[ OK ] OsTest.nonblock (0 ms)
[ RUN ] OsTest.touch
[ OK ] OsTest.touch (1 ms)
[ RUN ] OsTest.readWriteString
[ OK ] OsTest.readWriteString (0 ms)
[ RUN ] OsTest.find
[ OK ] OsTest.find (1 ms)
[ RUN ] OsTest.bootId
[ OK ] OsTest.bootId (0 ms)
[ RUN ] OsTest.uname
[ OK ] OsTest.uname (0 ms)
[ RUN ] OsTest.sysname
[ OK ] OsTest.sysname (0 ms)
[ RUN ] OsTest.release
[ OK ] OsTest.release (0 ms)
[ RUN ] OsTest.sleep
[ OK ] OsTest.sleep (11 ms)
[ RUN ] OsTest.pids
[ OK ] OsTest.pids (32 ms)
[ RUN ] OsTest.children
[ OK ] OsTest.children (52 ms)
[ RUN ] OsTest.process
[ OK ] OsTest.process (0 ms)
[ RUN ] OsTest.processes
[ OK ] OsTest.processes (17 ms)
[ RUN ] OsTest.killtree
[ OK ] OsTest.killtree (121 ms)
[ RUN ] OsTest.pstree
[ OK ] OsTest.pstree (24 ms)
[----------] 17 tests from OsTest (260 ms total)
[----------] 1 test from ProtobufTest
[ RUN ] ProtobufTest.JSON
[ OK ] ProtobufTest.JSON (1 ms)
[----------] 1 test from ProtobufTest (1 ms total)
[----------] 1 test from OsSendfileTest
[ RUN ] OsSendfileTest.sendfile
[ OK ] OsSendfileTest.sendfile (0 ms)
[----------] 1 test from OsSendfileTest (0 ms total)
[----------] 1 test from OsSignalsTest
[ RUN ] OsSignalsTest.suppress
[ OK ] OsSignalsTest.suppress (0 ms)
[----------] 1 test from OsSignalsTest (0 ms total)
[----------] 22 tests from StringsTest
[ RUN ] StringsTest.Format
[ OK ] StringsTest.Format (0 ms)
[ RUN ] StringsTest.Remove
[ OK ] StringsTest.Remove (0 ms)
[ RUN ] StringsTest.Replace
[ OK ] StringsTest.Replace (0 ms)
[ RUN ] StringsTest.Trim
[ OK ] StringsTest.Trim (0 ms)
[ RUN ] StringsTest.Tokenize
[ OK ] StringsTest.Tokenize (0 ms)
[ RUN ] StringsTest.TokenizeStringWithDelimsAtStart
[ OK ] StringsTest.TokenizeStringWithDelimsAtStart (0 ms)
[ RUN ] StringsTest.TokenizeStringWithDelimsAtEnd
[ OK ] StringsTest.TokenizeStringWithDelimsAtEnd (0 ms)
[ RUN ] StringsTest.TokenizeStringWithDelimsAtStartAndEnd
[ OK ] StringsTest.TokenizeStringWithDelimsAtStartAndEnd (0 ms)
[ RUN ] StringsTest.TokenizeWithMultipleDelims
[ OK ] StringsTest.TokenizeWithMultipleDelims (0 ms)
[ RUN ] StringsTest.TokenizeEmptyString
[ OK ] StringsTest.TokenizeEmptyString (0 ms)
[ RUN ] StringsTest.TokenizeDelimOnlyString
[ OK ] StringsTest.TokenizeDelimOnlyString (0 ms)
[ RUN ] StringsTest.TokenizeNullByteDelim
[ OK ] StringsTest.TokenizeNullByteDelim (0 ms)
[ RUN ] StringsTest.SplitEmptyString
[ OK ] StringsTest.SplitEmptyString (0 ms)
[ RUN ] StringsTest.SplitDelimOnlyString
[ OK ] StringsTest.SplitDelimOnlyString (0 ms)
[ RUN ] StringsTest.Split
[ OK ] StringsTest.Split (0 ms)
[ RUN ] StringsTest.SplitStringWithDelimsAtStart
[ OK ] StringsTest.SplitStringWithDelimsAtStart (0 ms)
[ RUN ] StringsTest.SplitStringWithDelimsAtEnd
[ OK ] StringsTest.SplitStringWithDelimsAtEnd (0 ms)
[ RUN ] StringsTest.SplitStringWithDelimsAtStartAndEnd
[ OK ] StringsTest.SplitStringWithDelimsAtStartAndEnd (0 ms)
[ RUN ] StringsTest.SplitWithMultipleDelims
[ OK ] StringsTest.SplitWithMultipleDelims (0 ms)
[ RUN ] StringsTest.Pairs
[ OK ] StringsTest.Pairs (0 ms)
[ RUN ] StringsTest.StartsWith
[ OK ] StringsTest.StartsWith (0 ms)
[ RUN ] StringsTest.Contains
[ OK ] StringsTest.Contains (0 ms)
[----------] 22 tests from StringsTest (0 ms total)
[----------] 1 test from Thread
[ RUN ] Thread.local
[ OK ] Thread.local (0 ms)
[----------] 1 test from Thread (0 ms total)
[----------] 1 test from UUIDTest
[ RUN ] UUIDTest.test
[ OK ] UUIDTest.test (1 ms)
[----------] 1 test from UUIDTest (1 ms total)
[----------] 6 tests from ProcTest
[ RUN ] ProcTest.pids
[ OK ] ProcTest.pids (0 ms)
[ RUN ] ProcTest.cpus
[ OK ] ProcTest.cpus (0 ms)
[ RUN ] ProcTest.SystemStatus
[ OK ] ProcTest.SystemStatus (0 ms)
[ RUN ] ProcTest.ProcessStatus
[ OK ] ProcTest.ProcessStatus (0 ms)
[ RUN ] ProcTest.SingleThread
[ OK ] ProcTest.SingleThread (1 ms)
[ RUN ] ProcTest.MultipleThreads
stout/tests/proc_tests.cpp:190: Failure
Value of: (((*(int *) &(status))) & 0x7f)
Actual: 4
Expected: 9
[ FAILED ] ProcTest.MultipleThreads (11 ms)
[----------] 6 tests from ProcTest (13 ms total)
[----------] Global test environment tear-down
[==========] 118 tests from 24 test cases ran. (394 ms total)
[ PASSED ] 117 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] ProcTest.MultipleThreads
1 FAILED TEST
make[7]: *** [check-local] Error 1
make[7]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make[6]: *** [check-am] Error 2
make[6]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make[5]: *** [check-recursive] Error 1
make[5]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make[4]: *** [check] Error 2
make[4]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess/3rdparty'
make[3]: *** [check-recursive] Error 1
make[3]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty/libprocess'
make[2]: *** [check-recursive] Error 1
make[2]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
make[1]: *** [check] Error 2
make[1]: Leaving directory `/x1/jenkins/jenkins-slave/workspace/mesos-reviewbot/3rdparty'
make: *** [check-recursive] Error 1
- Mesos ReviewBot
On April 3, 2014, 7:23 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 7:23 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/
-----------------------------------------------------------
(Updated April 3, 2014, 7:23 p.m.)
Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
Changes
-------
Entirely updated and rebased to address all comments.
Bugs: MESOS-1102
https://issues.apache.org/jira/browse/MESOS-1102
Repository: mesos-git
Description
-------
Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
Diffs (updated)
-----
3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
3rdparty/libprocess/src/subprocess.cpp e09c808
3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
Diff: https://reviews.apache.org/r/19259/diff/
Testing
-------
make check
Thanks,
Till Toenshoff
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
> On April 3, 2014, 4:18 a.m., Benjamin Hindman wrote:
> > This looks pretty good Till. Just a few cleanups and then let's get it submitted.
Thanks Ben!
> On April 3, 2014, 4:18 a.m., Benjamin Hindman wrote:
> > 3rdparty/libprocess/src/tests/subprocess_tests.cpp, lines 342-344
> > <https://reviews.apache.org/r/19259/diff/2/?file=520577#file520577line342>
> >
> > Knowing what you want to use this for, how about the following as a test. ;)
> >
> > int setup(const string& directory)
> > {
> > // Keep everything async-signal safe.
> > if (::chdir(directory.c_str()) == -1) {
> > return errno;
> > }
> >
> > if (::setsid() == -1) {
> > return errno;
> > }
> >
> > return 0;
> > }
> >
> >
> > TEST(Subprocess, setup)
> > {
> > Clock::pause();
> >
> > Try<string> directory = os::mkdtemp();
> > ASSERT_SOME(directory);
> >
> > Try<Subprocess> s = subprocess(
> > "echo hello world >file && sleep 60",
> > None(),
> > lambda::bind(&setup, directory.get()));
> >
> > ASSERT_SOME(s);
> >
> > // Verify the process is in a different session.
> > EXPECT_NE(getsid(0), getsid(s.get().pid()));
> >
> > kill(s.get().pid(), SIGTERM);
> >
> > // Advance time until the internal reaper reaps the subprocess.
> > while (s.get().status().isPending()) {
> > Clock::advance(Seconds(1));
> > Clock::settle();
> > }
> >
> > AWAIT_ASSERT_READY(s.get().status());
> > ASSERT_SOME(s.get().status().get());
> >
> > int status = s.get().status().get().get();
> > ASSERT_TRUE(WIFSIGNALED(status));
> > ASSERT_EQ(SIGTERM, WTERMSIG(status));
> >
> > // Make sure 'file' is there and contains 'hello world'.
> > const string& path = os::join(directory.get(), "file");
> > EXPECT_TRUE(os::exists(path));
> > EXPECT_SOME_EQ("hello world", os::read(path));
> >
> > os::rmdir(directory.get());
> >
> > Clock::resume();
> > }
Had to split this up as the SIGTERM was coming in too early, aborting/preventing the echo > file.
Also added another test for covering the 'setup' return value logic.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review39393
-----------------------------------------------------------
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Benjamin Hindman <be...@berkeley.edu>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review39393
-----------------------------------------------------------
This looks pretty good Till. Just a few cleanups and then let's get it submitted.
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/19259/#comment71792>
How about:
const Option<lambda::function<int(void)> >& setup = None());
Let's document this as a 'setup' function that can be run after forking but before executing the command that must be async-signal safe. Also mention that if it returns a non-zero error code that's what the status of the Subprocess will be (consider describing it as 'setup && command').
Also, let's make 'environment' be an Option now too since it'll be cleaner to pass 'None()' then an empty map.
3rdparty/libprocess/src/subprocess.cpp
<https://reviews.apache.org/r/19259/#comment71793>
Let's not do this as it's not standard in the code base and not really needed.
3rdparty/libprocess/src/tests/subprocess_tests.cpp
<https://reviews.apache.org/r/19259/#comment71794>
Knowing what you want to use this for, how about the following as a test. ;)
int setup(const string& directory)
{
// Keep everything async-signal safe.
if (::chdir(directory.c_str()) == -1) {
return errno;
}
if (::setsid() == -1) {
return errno;
}
return 0;
}
TEST(Subprocess, setup)
{
Clock::pause();
Try<string> directory = os::mkdtemp();
ASSERT_SOME(directory);
Try<Subprocess> s = subprocess(
"echo hello world >file && sleep 60",
None(),
lambda::bind(&setup, directory.get()));
ASSERT_SOME(s);
// Verify the process is in a different session.
EXPECT_NE(getsid(0), getsid(s.get().pid()));
kill(s.get().pid(), SIGTERM);
// Advance time until the internal reaper reaps the subprocess.
while (s.get().status().isPending()) {
Clock::advance(Seconds(1));
Clock::settle();
}
AWAIT_ASSERT_READY(s.get().status());
ASSERT_SOME(s.get().status().get());
int status = s.get().status().get().get();
ASSERT_TRUE(WIFSIGNALED(status));
ASSERT_EQ(SIGTERM, WTERMSIG(status));
// Make sure 'file' is there and contains 'hello world'.
const string& path = os::join(directory.get(), "file");
EXPECT_TRUE(os::exists(path));
EXPECT_SOME_EQ("hello world", os::read(path));
os::rmdir(directory.get());
Clock::resume();
}
- Benjamin Hindman
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Ritwik <ri...@gmail.com>.
I wonder if I should rather post this to the users' section.
On 25 March 2014 00:08, Mesos ReviewBot <de...@mesos.apache.org> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/#review38337
> -----------------------------------------------------------
>
>
> Bad patch!
>
> Reviews applied: [19416, 19162, 19259]
>
> Failed command: git apply --index 19259.patch
>
> Error:
> error: patch failed: 3rdparty/libprocess/src/subprocess.cpp:9
> error: 3rdparty/libprocess/src/subprocess.cpp: patch does not apply
>
>
> - Mesos ReviewBot
>
>
> On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
> >
> > -----------------------------------------------------------
> > This is an automatically generated e-mail. To reply, visit:
> > https://reviews.apache.org/r/19259/
> > -----------------------------------------------------------
> >
> > (Updated March 16, 2014, 11:05 a.m.)
> >
> >
> > Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon,
> and Ian Downes.
> >
> >
> > Bugs: MESOS-1102
> > https://issues.apache.org/jira/browse/MESOS-1102
> >
> >
> > Repository: mesos-git
> >
> >
> > Description
> > -------
> >
> > Adds the ability to process::subprocess to run a function (lambda)
> within the child context, after fork and before exec.
> >
> > NOTE: Such lambda must not contain any async unsafe code. For details on
> async safety, see POSIX.1-2004 on async-signal-safe functions, also
> referenced in the signal man-pages:
> http://man7.org/linux/man-pages/man7/signal.7.html.
> >
> >
> > Diffs
> > -----
> >
> > 3rdparty/libprocess/include/process/subprocess.hpp
> d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> > 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> > 3rdparty/libprocess/src/tests/subprocess_tests.cpp
> d15d4d159105474117c4ea432b215431209ab539
> >
> > Diff: https://reviews.apache.org/r/19259/diff/
> >
> >
> > Testing
> > -------
> >
> > make check
> >
> >
> > Thanks,
> >
> > Till Toenshoff
> >
> >
>
>
--
*Ritwik Yadav*
Department of Computer Science and Engineering,
Indian Institute of Technology,
Kharagpur.
Cell: +91-9635-152346
Twitter: @iRitwik <https://twitter.com/#%21/iRitwik>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Mesos ReviewBot <de...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review38337
-----------------------------------------------------------
Bad patch!
Reviews applied: [19416, 19162, 19259]
Failed command: git apply --index 19259.patch
Error:
error: patch failed: 3rdparty/libprocess/src/subprocess.cpp:9
error: 3rdparty/libprocess/src/subprocess.cpp: patch does not apply
- Mesos ReviewBot
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 75
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line75>
> >
> > It might be better to add a second method. The way this is written, anyone wanting to specify a child function will need to set the environment, which may not be useful.
> >
> > I propose a second method that takes a required function and an optional environment.
>
> Till Toenshoff wrote:
> Agreed.
>
> Benjamin Hindman wrote:
> I think as second method would be fine but alternatively just making the 'environment' be an Option is sufficient and a pattern used in other places in the code. IIUC the specific use case you're after requires both the 'environment' && 'setup' so let's make sure that we provide at least that overload right now.
Used Option<> on both, 'environment' and 'setup' as suggested - makes a lot of sense, I like the result.
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 79
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line79>
> >
> > Is there a reason this is restricted to void() methods only?
>
> Till Toenshoff wrote:
> What should happen if that method returned something? I briefly thought about using a bool instead and return if that was false. Would that be something you preferred?
>
> Ian Downes wrote:
> perhaps this should mimic the child function used in clone() on Linux, which returns int
Now returning an int. When that int is non-zero, it overrides the command status code.
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37433
-----------------------------------------------------------
On April 3, 2014, 7:23 p.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated April 3, 2014, 7:23 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp 6c51c0b
> 3rdparty/libprocess/src/subprocess.cpp e09c808
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp 9413ecc
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Ian Downes <ia...@gmail.com>.
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 79
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line79>
> >
> > Is there a reason this is restricted to void() methods only?
>
> Till Toenshoff wrote:
> What should happen if that method returned something? I briefly thought about using a bool instead and return if that was false. Would that be something you preferred?
perhaps this should mimic the child function used in clone() on Linux, which returns int
- Ian
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37433
-----------------------------------------------------------
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 75
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line75>
> >
> > It might be better to add a second method. The way this is written, anyone wanting to specify a child function will need to set the environment, which may not be useful.
> >
> > I propose a second method that takes a required function and an optional environment.
>
> Till Toenshoff wrote:
> Agreed.
I think as second method would be fine but alternatively just making the 'environment' be an Option is sufficient and a pattern used in other places in the code. IIUC the specific use case you're after requires both the 'environment' && 'setup' so let's make sure that we provide at least that overload right now.
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37433
-----------------------------------------------------------
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Till Toenshoff <to...@me.com>.
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 75
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line75>
> >
> > It might be better to add a second method. The way this is written, anyone wanting to specify a child function will need to set the environment, which may not be useful.
> >
> > I propose a second method that takes a required function and an optional environment.
Agreed.
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 77
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line77>
> >
> > why the rename from environment?
I guess I grabbed an outdated RR from your proposal (had to manually apply it.
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/include/process/subprocess.hpp, line 79
> > <https://reviews.apache.org/r/19259/diff/2/?file=520575#file520575line79>
> >
> > Is there a reason this is restricted to void() methods only?
What should happen if that method returned something? I briefly thought about using a bool instead and return if that was false. Would that be something you preferred?
> On March 17, 2014, 7:28 p.m., Dominic Hamon wrote:
> > 3rdparty/libprocess/src/subprocess.cpp, line 110
> > <https://reviews.apache.org/r/19259/diff/2/?file=520576#file520576line110>
> >
> > you have a 'using' - remove the lambda::
- Till
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37433
-----------------------------------------------------------
On March 16, 2014, 11:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 11:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>
Re: Review Request 19259: Added ability to process::subprocess to run
function within child context
Posted by Dominic Hamon <dh...@twopensource.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19259/#review37433
-----------------------------------------------------------
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/19259/#comment68960>
It might be better to add a second method. The way this is written, anyone wanting to specify a child function will need to set the environment, which may not be useful.
I propose a second method that takes a required function and an optional environment.
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/19259/#comment68959>
why the rename from environment?
3rdparty/libprocess/include/process/subprocess.hpp
<https://reviews.apache.org/r/19259/#comment68962>
Is there a reason this is restricted to void() methods only?
3rdparty/libprocess/src/subprocess.cpp
<https://reviews.apache.org/r/19259/#comment68963>
you have a 'using' - remove the lambda::
3rdparty/libprocess/src/tests/subprocess_tests.cpp
<https://reviews.apache.org/r/19259/#comment68964>
only two indent when you add the braces.
- Dominic Hamon
On March 16, 2014, 4:05 a.m., Till Toenshoff wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19259/
> -----------------------------------------------------------
>
> (Updated March 16, 2014, 4:05 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Ben Mahler, Dominic Hamon, and Ian Downes.
>
>
> Bugs: MESOS-1102
> https://issues.apache.org/jira/browse/MESOS-1102
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> Adds the ability to process::subprocess to run a function (lambda) within the child context, after fork and before exec.
>
> NOTE: Such lambda must not contain any async unsafe code. For details on async safety, see POSIX.1-2004 on async-signal-safe functions, also referenced in the signal man-pages: http://man7.org/linux/man-pages/man7/signal.7.html.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/include/process/subprocess.hpp d16cbc1e3d464e1784f116ccdb327cf0784f07c2
> 3rdparty/libprocess/src/subprocess.cpp PRE-CREATION
> 3rdparty/libprocess/src/tests/subprocess_tests.cpp d15d4d159105474117c4ea432b215431209ab539
>
> Diff: https://reviews.apache.org/r/19259/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Till Toenshoff
>
>