> 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)); > > ``` > > Greg Mann wrote: > Good catch, thanks! > > Jie Yu wrote: > Mpark reached out to me about this issue. Seems that this extra copying > is not necessary (my bad). The 'http' object will be bounded in the lambda > function and stored in Master's libprocess handlers map.
I checked out the IRC logs; that makes sense! I reverted back to passing `this`. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39988/#review106526 ----------------------------------------------------------- On Nov. 19, 2015, 11:41 p.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/39988/ > ----------------------------------------------------------- > > (Updated Nov. 19, 2015, 11:41 p.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 1c4f1406f5d917f5d655a7d61d311365f8999ce0 > src/master/master.hpp 5e5a575dc7dd49324f3c837028df8a7f75cd1f80 > 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 > >
