> On April 24, 2013, 7:34 p.m., Vinod Kone wrote:
> > src/tests/reaper_tests.cpp, lines 181-182
> > <https://reviews.apache.org/r/10746/diff/1/?file=283968#file283968line181>
> >
> >     instead of pulling this into a temp variable, just do 
> > processExited.get() (or status.get())

Can't, it causes error: "invalid lvalue in unary ‘&’".


> On April 24, 2013, 7:34 p.m., Vinod Kone wrote:
> > src/tests/reaper_tests.cpp, line 189
> > <https://reviews.apache.org/r/10746/diff/1/?file=283968#file283968line189>
> >
> >     You mean that has exited before that pid was asked to monitor, correct?

Yes. Changed the line to
"Check that the Reaper can monitor a child process that exits before monitor() 
is called on it."


> On April 24, 2013, 7:34 p.m., Vinod Kone wrote:
> > src/slave/reaper.cpp, lines 125-138
> > <https://reviews.apache.org/r/10746/diff/1/?file=283967#file283967line125>
> >
> >     Considering you always have the permission (because you reject 
> > non-permissible ones in monitor) is this necessary?
> >     
> >     Alternatively, is it correct to reject listeners in monitor() if this 
> > could still give them result?

Changed the logic so that in monitor() we not only check for alive() (which 
fails with insufficient permission) but also check (with waitpid) if pid is a 
child process. When either is satisfied, we add the pid to the map. 
If waitpid actually reaps a terminated pid in monitor() we have to return it 
directly. 

Then in reap(), all registered pids either 1) satisfy the permission check, or 
2) are children of the current process. Therefore we have two code blocks to 
deal with them. 


- Jiang Yan


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


On April 26, 2013, 7:53 p.m., Jiang Yan Xu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10746/
> -----------------------------------------------------------
> 
> (Updated April 26, 2013, 7:53 p.m.)
> 
> 
> Review request for mesos, Benjamin Hindman, Vinod Kone, and Ben Mahler.
> 
> 
> Description
> -------
> 
> See summary.
> 
> - Previously the listener was notified when its child processes terminate 
> whether it register them or not. 
> 
> 
> Diffs
> -----
> 
>   src/slave/cgroups_isolator.hpp f8fabc4e1c3c303b35a76db96b4b2479bd7c8ff8 
>   src/slave/cgroups_isolator.cpp 8b79da50d8fb0c2c8716dd7d2c734b65c32f60b4 
>   src/slave/process_isolator.hpp 9875f4a6e8e109e31ad390fbd7a84d03ad747190 
>   src/slave/process_isolator.cpp 6e2af87d291d7c3448393c1ffa816f7020e2dff6 
>   src/slave/reaper.hpp 09844d8d47b143ee369e0c82b19d65a774df4a90 
>   src/slave/reaper.cpp bd3dcef07c370ad338b478755bf8f7ce6408e4a3 
>   src/tests/reaper_tests.cpp 0809c1ff17eb949beb1bdd922fdced022aa202f3 
>   src/tests/utils.hpp ffe637f2f03ff5ca020a4d2cb617be047aade034 
> 
> Diff: https://reviews.apache.org/r/10746/diff/
> 
> 
> Testing
> -------
> 
> make check.
> 
> 
> Thanks,
> 
> Jiang Yan Xu
> 
>

Reply via email to