Don't capture AsyncLocals to PhysicalFilesWatcher - #334
Conversation
| _fileWatcher.Renamed += OnRenamed; | ||
| _fileWatcher.Deleted += OnChanged; | ||
| _fileWatcher.Error += OnError; | ||
| // Don't capture the current ExecutionContext and its AsyncLocals onto the token registration causing them to live forever |
|
Will only capture on linux (due to cancellation token) issue on Windows is due to the ChangeTokens (addressed elsewhere) |
where is this addressed? because this one is closed. |
|
For linux I raised an issue https://github.com/dotnet/corefx/issues/31973 However all the FileSystemWatcher roots on your trace should be fixed by dotnet/extensions#392 |
|
@benaadams this should be fixed by the use of 32dbcac. |
fb52f20 to
432a195
Compare
Different capturing; its captured onto the FileSystemWatcher when the events are enabled with Unfortunately I can't write a test for it as ASP.NET Core now registers the callback as a cancellation token with the flow suppressed dotnet/extensions#392; so while it captures it; it doesn't flow it back to be observable :-/ /cc @stephentoub |
| restoreFlow = true; | ||
| } | ||
|
|
||
| // Perf: Turn off the file monitoring if no files to monitor. |
|
Thank you for your effort. As part of the effort to reduce the number of repositories we're going to move the code of this repo to https://github.com/aspnet/aspnetcore repo. Feel free to resubmit a new PR in the target repo as we're closing this PR now. |
4 participants
This causes a problem with
IHttpContextAccessoras it captures theHttpContextand everything linked to it causing a lot of memory to become rooted (as seen in https://github.com/aspnet/KestrelHttpServer/issues/2840#issuecomment-416034872)/cc @davidfowl