> On Aug. 15, 2016, 7:30 p.m., Benjamin Mahler wrote:
> > src/slave/posix_signalhandler.hpp, lines 39-47
> > <https://reviews.apache.org/r/51122/diff/1/?file=1474885#file1474885line39>
> >
> >     This note seems to suggest that we should only set up the signal 
> > handler from main.cpp?
> >     
> >     If the behavior is undefined, seems we should CHECK against this?

The behavior is undefined in the sense that signals will only be handled by one 
agent within the OS process.  If there are multiple agents (i.e. if using 
`mesos-local`) and we send a `SIGUSR1`, only the last agent will shutdown.

It would make sense to install the handlers for `main.cpp` and guard this 
function with a check.  As it turns out, the `Slave::signaled` function is 
already public due to this TODO:
https://github.com/apache/mesos/blob/a86a145770f22273caeb15953aaff33acd2eba42/src/slave/slave.hpp#L300-L303

The signaled function is defined here:
https://github.com/apache/mesos/blob/a86a145770f22273caeb15953aaff33acd2eba42/src/slave/slave.hpp#L392-L393

I'll make the change...


- Joseph


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


On Aug. 15, 2016, 6:34 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51122/
> -----------------------------------------------------------
> 
> (Updated Aug. 15, 2016, 6:34 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Greg Mann, Artem Harutyunyan, and 
> Vinod Kone.
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This leak was introduced on purpose in:
> https://reviews.apache.org/r/34016/
> 
> When we run tests in repetition, each agent will create a new signal
> handler and leak the old one.  We can either delete the old signal
> handler, or guard this function with a `Once`.
> 
> 
> Diffs
> -----
> 
>   src/slave/posix_signalhandler.hpp 4a5411793e55f912c6e8ae72f9a8042d3a6195b5 
> 
> Diff: https://reviews.apache.org/r/51122/diff/
> 
> 
> Testing
> -------
> 
> make check (OSX)
> 
> Found this leak via code inspection after valgrind.
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>

Reply via email to