Use async ChangeToken.OnChange overload in FileConfigurationProvider#130492
Use async ChangeToken.OnChange overload in FileConfigurationProvider#130492svick wants to merge 1 commit into
Conversation
Replace the blocking Thread.Sleep in the reload callback with an awaited Task.Delay, using the new async Func<Task> overload of ChangeToken.OnChange (dotnet#69099). This avoids blocking a thread for the duration of the reload delay. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 059bf1b8-31e2-4608-a038-210880f90d06
|
Tagging subscribers to this area: @dotnet/area-extensions-configuration |
There was a problem hiding this comment.
Pull request overview
This PR updates FileConfigurationProvider’s file-change reload callback to use the async ChangeToken.OnChange overload and replace a blocking Thread.Sleep debounce with an awaited Task.Delay, aiming to avoid tying up a thread during the reload delay.
Changes:
- Replace
Thread.Sleep(Source.ReloadDelay)withawait Task.Delay(Source.ReloadDelay).ConfigureAwait(false)inside the reload callback. - Switch the callback passed to
ChangeToken.OnChangeto anasynclambda (binding to theFunc<Task>overload). - Update
usingdirectives accordingly (System.Threading→System.Threading.Tasks).
| _changeTokenRegistration = ChangeToken.OnChange( | ||
| () => Source.FileProvider.Watch(Source.Path!), | ||
| () => | ||
| async () => | ||
| { | ||
| Thread.Sleep(Source.ReloadDelay); | ||
| await Task.Delay(Source.ReloadDelay).ConfigureAwait(false); | ||
| Load(reload: true); | ||
| }); |
There was a problem hiding this comment.
In the synchronous case, the exception is rethrown in whatever the IFileProvider uses for watching. In case it's PhysicalFilesWatcher, the exception is swallowed anyway.
So I don't think this is a problem.
There was a problem hiding this comment.
It holds specifically for PhysicalFilesWatcher (the provider behind AddJsonFile / AddXmlFile etc.). A hypothetical custom IFileProvider that invokes token callbacks synchronously and lets exceptions propagate would surface the error in the sync model but not the async one. That is within the async overload's documented contract ("asynchronous exceptions are left unobserved"). I know this is minor issue but is it possible?
Uses the new async (
Func<Task>) overload ofChangeToken.OnChange(see #69099) inFileConfigurationProvider.The reload callback previously blocked a thread for the whole reload delay via
Thread.Sleep(Source.ReloadDelay). It nowawaitsTask.Delay(...)instead, so no thread is blocked while waiting out the debounce delay before reloading:Why only this call site
The other in-repo callers of
ChangeToken.OnChange(ConfigurationRoot,ConfigurationManager,OptionsMonitor) invoke short, synchronous, non-blocking consumers (RaiseChanged/InvokeChanged) and gain nothing from the async overload, so they are intentionally left unchanged.A closely related change is at dotnet/aspnetcore#67732.
Note
This PR description was generated by GitHub Copilot.