mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Bannier <benjamin.bann...@mesosphere.io>
Subject Re: Review Request 46319: Added authorization to agents' `/monitor/statistics` endpoints.
Date Tue, 26 Apr 2016 08:00:16 GMT


> On April 22, 2016, 10:22 a.m., Adam B wrote:
> > src/slave/slave.hpp, lines 471-472
> > <https://reviews.apache.org/r/46319/diff/2/?file=1348482#file1348482line471>
> >
> >     How did this come up? The original `_statistics()` is not static, and it had
no issues.
> 
> Benjamin Bannier wrote:
>     The original `statistics` did not make any use of member data in any of the `defer`ed
actions (all the used data came from some `Future<ResourceUsage>` directly passed into
the callback; the call of `slave->self()` was not executed in the `defer`ed action). Now
here we need to access a rate limiter which is a member of `Http` which in turn might go away
before `_statistics` is executed.
> 
> Adam B wrote:
>     And now that `_statistics()` is synchronous and no longer uses the limiter, do we
still need to worry about this?

Even if we do not use any member data, it is still undefined behavior to invoke a non-static
member function on a destroyed object.


- Benjamin


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


On April 26, 2016, 9:59 a.m., Benjamin Bannier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46319/
> -----------------------------------------------------------
> 
> (Updated April 26, 2016, 9:59 a.m.)
> 
> 
> Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht.
> 
> 
> Bugs: MESOS-5164
>     https://issues.apache.org/jira/browse/MESOS-5164
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added authorization to agents' `/monitor/statistics` endpoints.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp 537736d1fe42e8150bad91326299ef9a17041a8e 
>   src/slave/slave.hpp 57b18882e30e44dcc40449b0e3be8ee970c45bc8 
>   src/tests/slave_authorization_tests.cpp PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/46319/diff/
> 
> 
> Testing
> -------
> 
> make check (OS X, clang w/o optimization)
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message