> On March 27, 2015, 10:46 p.m., Adam B wrote: > > Fix looks good to me. Buildbot, please take another look. ;) > > Vinod Kone wrote: > Can you or @till let me know what the race and temporal issue here was? > And how this fixes it?
There was one major problem in these lines: ``` // Start authentication. const Future<Option<string>>& future = authenticator.get()->authenticate(from) .onAny(defer(self(), &Self::_authenticate, pid, lambda::_1)); ``` The returned future was destructed before the local reference (`future`) would even receive it. On the left side, we have a const ref. On the right side, we have a temporal (returned by `authenticate`). This in itself is fine as the lifetime of the temporary will be extended by the lifetime of the reference. Trouble is that the expression `onAny` then gets called on a reference to a temporary and generates a reference to that on return. The returned reference will not extend the lifetime of the future. Now havoc breaks lose as we have a dangling reference. The problem is not showing on all systems as they OS may not immediately reuse the destroyed objects' memory. For the "possible race" - that actually was not really a race - my update-comment was misleading. It was the fact that I realized that an onAny/... assignment could immediately trigger a callback invocation. Our "defer" certainly delays the callback but I felt that it would look much cleaner if I made sure that the initializing of "authenticating" happened before assigning the callback. - Till ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review78123 ----------------------------------------------------------- On March 25, 2015, 11:35 p.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated March 25, 2015, 11:35 p.m.) > > > Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. > > > Bugs: MESOS-2050 > https://issues.apache.org/jira/browse/MESOS-2050 > > > Repository: mesos > > > Description > ------- > > The initial design and implementation of the authenticator module interface > caused issues and was not optimal for heavy lifting setup of external > dependencies. By introducing a two fold design, this has been decoupled from > the authentication message processing. The new design also gets us back on > track to the goal of makeing SASL a soft dependency of mesos. > > > Diffs > ----- > > include/mesos/authentication/authenticator.hpp f66217a > src/authentication/cram_md5/authenticator.hpp c6f465f > src/authentication/cram_md5/authenticator.cpp PRE-CREATION > src/authentication/cram_md5/auxprop.hpp b894386 > src/master/master.hpp 3c957ab > src/master/master.cpp dccd7c6 > src/tests/cram_md5_authentication_tests.cpp 92a89c5 > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
