> On Nov. 13, 2015, 11:38 p.m., Jie Yu wrote: > > src/master/http.cpp, line 853 > > <https://reviews.apache.org/r/39988/diff/6/?file=1125444#file1125444line853> > > > > This looks problematic to me. 'this' will become invalid once this > > function returns. That means when `_reserve` is actually called after the > > authorization is done, it might be a dangling pointer. > > > > Instead, you can try to copy the http object (something like below): > > > > ``` > > lambda::function<Future<Response>(bool)> _reserve = > > lambda::bind( > > &Master::Http::_operation, > > *this, > > slaveId, > > resources.flatten(), > > operation, > > lambda::_1); > > > > return master->authorizeReserveResources(operation.reserve(), principal) > > .then(defer(master->self(), _reserve)); > > ```
Good catch, thanks! - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39988/#review106526 ----------------------------------------------------------- On Nov. 14, 2015, 12:01 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39988/ > ----------------------------------------------------------- > > (Updated Nov. 14, 2015, 12:01 a.m.) > > > Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff. > > > Bugs: MESOS-3062 > https://issues.apache.org/jira/browse/MESOS-3062 > > > Repository: mesos > > > Description > ------- > > Added authorization for dynamic reservation master endpoints. > Note: this review is continued from https://reviews.apache.org/r/37126/ > > > Diffs > ----- > > src/master/http.cpp f4ec23d74e203b2a8f2af187f0e56fbde7d9b3e5 > src/master/master.hpp ead8520b7108a0f2c3a0bb11ae7b543897d111a2 > src/tests/reservation_endpoints_tests.cpp > 1552e4537c4f4d79bfa4bc17ccab2df630bc32a4 > > Diff: https://reviews.apache.org/r/39988/diff/ > > > Testing > ------- > > This is the fourth in a chain of 5 patches. Added new reservation endpoints > tests to validate authorization of reserve and unreserve operations using > ACLs. `make check` was run to test after all patches were applied. > > > Thanks, > > Greg Mann > >
