-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58464/#review172046
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58327, 58328, 58251, 58252, 58253, 58254, 58255, 58258, 58428, 58458, 58464]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose'
ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On April 14, 2017, 11:22 p.m., Greg Mann wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58464/
> -----------------------------------------------------------
>
> (Updated April 14, 2017, 11:22 p.m.)
>
>
> Review request for mesos, Anand Mazumdar and Vinod Kone.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This patch fixes a bug in `CombinedAuthenticator.authenticate()`.
> Previously, the function registered a callback on the results of
> `Authenticator::authenticate()` calls, which would be executed in
> the context of the individual authenticators' processes. Since this
> callback captured a copy of the `Owned<Authenticator>` referring to
> that authenticator itself, it was possible that during teardown the
> authenticator could be destroyed from its own context, leading to
> a deadlock.
>
> This patch eliminates the capture of a reference to the
> authenticator, instead simply capturing the authenticator's scheme.
>
>
> Diffs
> -----
>
> src/authentication/http/combined_authenticator.cpp 51ec8829cd3323f23389537d5fce4850ffcfc0e9
>
>
> Diff: https://reviews.apache.org/r/58464/diff/1/
>
>
> Testing
> -------
>
> `bin/mesos-tests.sh --gtest_filter="ExecutorAuthorizationTest.RunTaskGroup" --gtest_repeat=-1
--gtest_break_on_failure` was executed both before and after this patch to verify that it
eliminates the deadlock.
>
>
> Thanks,
>
> Greg Mann
>
>
|