Fix logger initialization continuation race in LoggingBus#8006
Conversation
StartDefaultLoggers was only waiting for the Ask task (logger ACK), not the continuation that publishes "Logger started" message. This caused the continuation to run during test teardown, triggering EventFilter assertions. The fix returns the continuation task from AddLogger and uses TaskContinuationOptions.ExecuteSynchronously to ensure the continuation runs immediately when the Ask completes, before WaitAll returns.
Aaronontheweb
left a comment
There was a problem hiding this comment.
I should point out that even though this fix touches core Akka, it's mostly just a problem for our test suite.
| }); | ||
| return (askTask, fullLoggerName); | ||
| }, TaskContinuationOptions.ExecuteSynchronously); | ||
| return (continuationTask, fullLoggerName); |
There was a problem hiding this comment.
Return the continuation task and have waiters wait on that, versus the original task itself.
There was a problem hiding this comment.
Just noticed this now, but if askTask threw, the continuation would fail on var response = t.Result; and the logger would not be removed/unsubscribed. Maybe we need to check for t.IsFaulted too?
Arkatufus
left a comment
There was a problem hiding this comment.
Comment not relayed to the actual fix
| }); | ||
| return (askTask, fullLoggerName); | ||
| }, TaskContinuationOptions.ExecuteSynchronously); | ||
| return (continuationTask, fullLoggerName); |
There was a problem hiding this comment.
Just noticed this now, but if askTask threw, the continuation would fail on var response = t.Result; and the logger would not be removed/unsubscribed. Maybe we need to check for t.IsFaulted too?
Handle the case where the Ask operation fails with an exception, not just when it's cancelled. This ensures the logger is properly removed and unsubscribed if initialization throws an exception.
| } | ||
|
|
||
|
|
||
| // Ask operation failed with an exception |
…#8006) * Fix logger initialization continuation race in LoggingBus StartDefaultLoggers was only waiting for the Ask task (logger ACK), not the continuation that publishes "Logger started" message. This caused the continuation to run during test teardown, triggering EventFilter assertions. The fix returns the continuation task from AddLogger and uses TaskContinuationOptions.ExecuteSynchronously to ensure the continuation runs immediately when the Ask completes, before WaitAll returns. * Add IsFaulted check for logger initialization Handle the case where the Ask operation fails with an exception, not just when it's cancelled. This ensures the logger is properly removed and unsubscribed if initialization throws an exception.
* Fix logger initialization continuation race in LoggingBus StartDefaultLoggers was only waiting for the Ask task (logger ACK), not the continuation that publishes "Logger started" message. This caused the continuation to run during test teardown, triggering EventFilter assertions. The fix returns the continuation task from AddLogger and uses TaskContinuationOptions.ExecuteSynchronously to ensure the continuation runs immediately when the Ask completes, before WaitAll returns. * Add IsFaulted check for logger initialization Handle the case where the Ask operation fails with an exception, not just when it's cancelled. This ensures the logger is properly removed and unsubscribed if initialization throws an exception.
Summary
Fixes flaky EventFilterDebugTests where "Logger started" message was published during test teardown.
Problem
StartDefaultLoggerswas only waiting for the Ask task (logger ACK), not the continuation that publishes "Logger started" message. This caused the continuation to run during test teardown, triggering EventFilter assertions.Solution
The fix makes two changes to
AddLogger:Task.WaitAllwaits for the full initialization sequenceTaskContinuationOptions.ExecuteSynchronouslyto ensure the continuation runs immediately when the Ask completesChanges
src/core/Akka/Event/LoggingBus.cs: Return continuation task fromAddLogger()and addExecuteSynchronouslyoptionTest plan
EventFilterDebugTests.Messages_can_be_muted_from_now_on_with_usingpasses 10/10 runs