Skip to content

Commit d4aa3fa

Browse files
AarononthewebAaronontheweb
authored
harden BackoffSupervisorSpecs (#7420)
* BackoffSupervisorSpec: fix `Watch` calls to `WatchAsync` * prevent race condition with competing `Termination` messages * add `AwaitAssert` * fixed more race conditions
1 parent 2a82323 commit d4aa3fa

File tree

1 file changed

+76
-56
lines changed

1 file changed

+76
-56
lines changed

‎src/core/Akka.Tests/Pattern/BackoffSupervisorSpec.cs‎

Lines changed: 76 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public async Task BackoffSupervisor_must_start_child_again_when_it_stops_when_us
9898
var supervisor = Create(OnStopOptions());
9999
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
100100
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
101-
Watch(c1);
101+
await WatchAsync(c1);
102102
c1.Tell(PoisonPill.Instance);
103103
await ExpectTerminatedAsync(c1);
104104
await AwaitAssertAsync(async() =>
@@ -125,21 +125,6 @@ public async Task BackoffSupervisor_must_forward_messages_to_the_child()
125125
[Fact]
126126
public async Task BackoffSupervisor_must_support_custom_supervision_strategy()
127127
{
128-
Func<IActorRef, Task> assertCustomStrategy = async supervisor =>
129-
{
130-
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
131-
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
132-
Watch(c1);
133-
c1.Tell("boom");
134-
await ExpectTerminatedAsync(c1);
135-
await AwaitAssertAsync(async () =>
136-
{
137-
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
138-
// new instance
139-
(await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref.Should().NotBeSameAs(c1);
140-
});
141-
};
142-
143128
// TODO: use FilterException
144129
await EventFilter.Exception<TestException>().ExpectAsync(2, async () =>
145130
{
@@ -163,9 +148,25 @@ await EventFilter.Exception<TestException>().ExpectAsync(2, async () =>
163148
return Directive.Escalate;
164149
});
165150

166-
await assertCustomStrategy(Create(OnStopOptions().WithSupervisorStrategy(stoppingStrategy)));
167-
await assertCustomStrategy(Create(OnFailureOptions().WithSupervisorStrategy(restartingStrategy)));
151+
await AssertCustomStrategy(Create(OnStopOptions().WithSupervisorStrategy(stoppingStrategy)));
152+
await AssertCustomStrategy(Create(OnFailureOptions().WithSupervisorStrategy(restartingStrategy)));
168153
});
154+
return;
155+
156+
async Task AssertCustomStrategy(IActorRef supervisor)
157+
{
158+
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
159+
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
160+
await WatchAsync(c1);
161+
c1.Tell("boom");
162+
await ExpectTerminatedAsync(c1);
163+
await AwaitAssertAsync(async () =>
164+
{
165+
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
166+
// new instance
167+
(await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref.Should().NotBeSameAs(c1);
168+
});
169+
}
169170
}
170171

171172
[Fact]
@@ -177,7 +178,7 @@ await EventFilter.Exception<TestException>().ExpectAsync(1, async () =>
177178
var supervisor = Create(OnStopOptions().WithDefaultStoppingStrategy().WithManualReset());
178179
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
179180
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
180-
Watch(c1);
181+
await WatchAsync(c1);
181182
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
182183
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(0);
183184

@@ -279,7 +280,7 @@ await EventFilter.Exception<TestException>().ExpectAsync(1, async() =>
279280
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
280281

281282
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
282-
Watch(c1);
283+
await WatchAsync(c1);
283284
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
284285
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(0);
285286

@@ -306,7 +307,7 @@ await EventFilter.Exception<TestException>().ExpectAsync(1, async() =>
306307
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
307308

308309
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
309-
Watch(c1);
310+
await WatchAsync(c1);
310311
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
311312
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(0);
312313

@@ -365,33 +366,42 @@ await AwaitConditionAsync(async() =>
365366
return (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
366367
}
367368

368-
Watch(supervisor);
369+
await WatchAsync(supervisor);
369370

370371
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
371372
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(0);
372373

373374
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
374375
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
375-
Watch(c1);
376+
await WatchAsync(c1);
376377
c1.Tell(PoisonPill.Instance);
377378
await ExpectTerminatedAsync(c1);
378379

379-
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
380-
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(1);
380+
// have to spin here because our message might get processed first, before the BackoffSupervisor can do its work
381+
await AwaitAssertAsync(async () =>
382+
{
383+
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
384+
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(1);
385+
});
386+
381387

382388
// This code looks suspicious, this might be the cause of the raciness
383389
var c2 = await WaitForChild();
384390
await AwaitAssertAsync(() => c2.ShouldNotBe(c1));
385-
Watch(c2);
391+
await WatchAsync(c2);
386392
c2.Tell(PoisonPill.Instance);
387393
await ExpectTerminatedAsync(c2);
388394

389-
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
390-
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(2);
395+
// have to spin here because our message might get processed first, before the BackoffSupervisor can do its work
396+
await AwaitAssertAsync(async () =>
397+
{
398+
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
399+
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(2);
400+
});
391401

392402
var c3 = await WaitForChild();
393403
await AwaitAssertAsync(() => c3.ShouldNotBe(c2));
394-
Watch(c3);
404+
await WatchAsync(c3);
395405
c3.Tell(PoisonPill.Instance);
396406
await ExpectTerminatedAsync(c3);
397407
await ExpectTerminatedAsync(supervisor);
@@ -403,50 +413,60 @@ public async Task BackoffSupervisor_must_stop_restarting_the_child_after_reachin
403413
await EventFilter.Exception<TestException>().ExpectAsync(3, async() =>
404414
{
405415
var supervisor = Create(OnFailureOptions(maxNrOfRetries: 2));
406-
407-
async Task<IActorRef> WaitForChild()
408-
{
409-
await AwaitConditionAsync(async () =>
410-
{
411-
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
412-
var c = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
413-
return !c.IsNobody();
414-
}, TimeSpan.FromSeconds(1), TimeSpan.FromMilliseconds(50));
415-
416-
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
417-
return (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
418-
}
419-
420-
Watch(supervisor);
416+
var supervisorProbe = CreateTestProbe();
417+
await supervisorProbe.WatchAsync(supervisor);
421418

422419
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
423420
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(0);
424421

425422
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
426423
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
427-
Watch(c1);
424+
await WatchAsync(c1);
428425
c1.Tell("boom");
429426
await ExpectTerminatedAsync(c1);
430427

431-
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
432-
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(1);
428+
// have to spin here because our message might get processed first, before the BackoffSupervisor can do its work
429+
await AwaitAssertAsync(async () =>
430+
{
431+
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
432+
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(1);
433+
});
433434

434435
// This code looks suspicious, this might be the cause of the raciness
435436
var c2 = await WaitForChild();
436437
await AwaitAssertAsync(() => c2.ShouldNotBe(c1));
437-
Watch(c2);
438+
await WatchAsync(c2);
438439
c2.Tell("boom");
439440
await ExpectTerminatedAsync(c2);
440441

441-
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
442-
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(2);
442+
443+
// have to spin here because our message might get processed first, before the BackoffSupervisor can do its work
444+
await AwaitAssertAsync(async () =>
445+
{
446+
supervisor.Tell(BackoffSupervisor.GetRestartCount.Instance);
447+
(await ExpectMsgAsync<BackoffSupervisor.RestartCount>()).Count.Should().Be(2);
448+
});
443449

444450
var c3 = await WaitForChild();
445451
await AwaitAssertAsync(() => c3.ShouldNotBe(c2));
446-
Watch(c3);
452+
await WatchAsync(c3);
447453
c3.Tell("boom");
448454
await ExpectTerminatedAsync(c3);
449-
await ExpectTerminatedAsync(supervisor);
455+
await supervisorProbe.ExpectTerminatedAsync(supervisor);
456+
return;
457+
458+
async Task<IActorRef> WaitForChild()
459+
{
460+
await AwaitConditionAsync(async () =>
461+
{
462+
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
463+
var c = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
464+
return !c.IsNobody();
465+
}, TimeSpan.FromSeconds(1), TimeSpan.FromMilliseconds(50));
466+
467+
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
468+
return (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
469+
}
450470
});
451471
}
452472

@@ -458,8 +478,8 @@ public async Task BackoffSupervisor_must_stop_restarting_the_child_if_final_stop
458478
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
459479
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
460480
var parentSupervisor = CreateTestProbe();
461-
Watch(c1);
462-
parentSupervisor.Watch(supervisor);
481+
await WatchAsync(c1);
482+
await parentSupervisor.WatchAsync(supervisor);
463483

464484
supervisor.Tell(stopMessage);
465485
await ExpectMsgAsync("stop");
@@ -476,8 +496,8 @@ public async Task BackoffSupervisor_must_not_stop_when_final_stop_message_has_no
476496
var supervisor = Create(OnStopOptions(maxNrOfRetries: 100).WithFinalStopMessage(message => ReferenceEquals(message, stopMessage)));
477497
supervisor.Tell(BackoffSupervisor.GetCurrentChild.Instance);
478498
var c1 = (await ExpectMsgAsync<BackoffSupervisor.CurrentChild>()).Ref;
479-
Watch(c1);
480-
supervisorWatcher.Watch(supervisor);
499+
await WatchAsync(c1);
500+
await supervisorWatcher.WatchAsync(supervisor);
481501

482502
c1.Tell(PoisonPill.Instance);
483503
await ExpectTerminatedAsync(c1);

0 commit comments

Comments
 (0)