-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fixes race condition with AskTimeoutSpec #2450
fixes race condition with AskTimeoutSpec #2450
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Described my changes.
@@ -56,16 +56,24 @@ public AskTimeoutSpec() | |||
var actor = Sys.ActorOf<SleepyActor>(); | |||
|
|||
var actorCell = actor as ActorRefWithCell; | |||
Assert.NotNull(actorCell); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly to make ReSharper compiler warnings go away / throw the correct assertion error in the event that the underlying ActorCell
is modified in a meaningful way in the future.
@@ -826,7 +826,8 @@ public override IActorRef GetChild(IEnumerable<string> name) | |||
} | |||
|
|||
/// <summary> | |||
/// TBD | |||
/// Returns <c>true</c> if the <see cref="VirtualPathContainer"/> contains any children, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filled in relevant comments.
@@ -160,7 +160,7 @@ internal static IActorRefProvider ResolveProvider(ICanTell self) | |||
timeoutCancellation.Cancel(); | |||
timeoutCancellation.Dispose(); | |||
} | |||
for (int i = 0; i < ctrList.Count; i++) | |||
for (var i = 0; i < ctrList.Count; i++) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Style
container.ForEachChild(x => childCounter++); | ||
Assert.True(childCounter==0,"Number of children in temp container should be 0."); | ||
// Need to spin here, since the continuation function may not execute immediately | ||
AwaitAssert(() => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actual fix: the FutureActorRef
is not unregistered from the VirtualPathContainer
until after a continuation function has a chance to run, hence we need an AwaitAssert
here in case that continuation hasn't been scheduled by the time we perform the assertion the first time around.
close #2449