-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix logger initialization continuation race in LoggingBus #8006
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
Changes from all commits
c9a12ca
1925154
34d40bf
a17f939
bfcae70
111e290
7860a44
1e9ec60
819bca2
c1e7efd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -204,8 +204,10 @@ private void RemoveLogger(IActorRef logger) | |
| var fullLoggerName = $"{loggerName} [{loggerType.FullName}]"; | ||
| var logger = system.SystemActorOf(Props.Create(loggerType).WithDispatcher(system.Settings.LoggersDispatcher), loggerName); | ||
| var askTask = logger.Ask(new InitializeLogger(this), Timeout.InfiniteTimeSpan, _shutdownCts.Token); | ||
|
|
||
| askTask.ContinueWith(t => | ||
|
|
||
| // Return the continuation task, not the ask task, so callers wait for | ||
| // the full initialization sequence including the "Logger started" message | ||
| var continuationTask = askTask.ContinueWith(t => | ||
| { | ||
| // _shutdownCts was cancelled while this logger is still loading | ||
| if (t.IsCanceled) | ||
|
|
@@ -215,7 +217,16 @@ private void RemoveLogger(IActorRef logger) | |
| RemoveLogger(logger); | ||
| return; | ||
| } | ||
|
|
||
|
|
||
| // Ask operation failed with an exception | ||
| if (t.IsFaulted) | ||
| { | ||
| Publish(new Error(t.Exception, loggingBusName, GetType(), | ||
| $"Logger {fullLoggerName} failed to respond to initialization request. Stopping logger.")); | ||
| RemoveLogger(logger); | ||
| return; | ||
| } | ||
|
|
||
| // Task ran to completion successfully | ||
| var response = t.Result; | ||
| if (response is not LoggerInitialized) | ||
|
|
@@ -231,8 +242,8 @@ private void RemoveLogger(IActorRef logger) | |
| _loggers.Add(logger); | ||
| SubscribeLogLevelAndAbove(LogLevel, logger); | ||
| Publish(new Debug(loggingBusName, GetType(), $"Logger {fullLoggerName} started")); | ||
| }); | ||
| return (askTask, fullLoggerName); | ||
| }, TaskContinuationOptions.ExecuteSynchronously); | ||
| return (continuationTask, fullLoggerName); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Return the continuation task and have waiters wait on that, versus the original task itself.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just noticed this now, but if |
||
| } | ||
|
|
||
| private string CreateLoggerName(Type actorClass) | ||
|
|
||
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.
Do this address your concern @Arkatufus ?
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.
LGTM