0

I've written a simple debouncer for use with ASP.NET Core 5 (to detect changes to config files). This will be used within the Startup class. It works well.

However I get an analyzer warning:

Do not create tasks without passing a TaskScheduler csharp(CA2008)

The problem code:

Task.Delay(TimeSpan.FromSeconds(2)).ContinueWith(
  task => someAction(),
  cancellationToken
);

Which of the following I should use, and how do they differ?

Task.Delay(TimeSpan.FromSeconds(2)).ContinueWith(
  task => someAction(),
  cancellationToken,
  TaskContinuationOptions.None,
  TaskScheduler.FromCurrentSynchronizationContext()             // <---
);

Task.Delay(TimeSpan.FromSeconds(2)).ContinueWith(
  task => someAction(),
  cancellationToken,
  TaskContinuationOptions.None,
  TaskScheduler.Default                                          // <---
);
lonix
  • 14,255
  • 23
  • 85
  • 176
  • ASP.NET Core does not use `SynchronizationContext`. Please read [Stephen Cleary's article](https://blog.stephencleary.com/2017/03/aspnetcore-synchronization-context.html) – Peter Csala Mar 04 '21 at 13:13
  • You should use `TaskScheduler.Default` – Matthew Watson Mar 04 '21 at 13:14
  • @MatthewWatson Thanks, if you add that as an answer I'll close :) – lonix Mar 04 '21 at 13:15
  • @lonix why change the task scheduler? Or use `ContinueWith`? The default (returned by TaskScheduler.Default) works fine. `await Task.Delay(TimeSpan.FromSeconds(2));` works fine as well – Panagiotis Kanavos Mar 04 '21 at 13:16
  • @PanagiotisKanavos I wouldn't have even considered this if it wasn't for `CA2008`, so I just want to use whatever is the safe default in aspnet core, unfortunately I don't know what that is. I keep reading that this is a source for error if not done properly, hence my question. – lonix Mar 04 '21 at 13:18
  • @lonix this rule [complains about ContinueWith](https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/quality-rules/ca2008) not `Task.Delay`. Why don't you use `await` ? ASP.NET Core has no `SynchronizationContext` so `await` isn't going to block – Panagiotis Kanavos Mar 04 '21 at 13:19
  • @PanagiotisKanavos I wish I could do that - but its called from `Startup.Configure()` which isn't async. – lonix Mar 04 '21 at 13:22
  • @lonix in that case you have a more serious bug - calling async code in a sync method. At some point you'll have to block and *that's* what can problems. I could even say that `Configure` is *not* the place to include logic and async operations. If you want to include config settings from a remote source, create a separate configuration provider and implement its asynchronous operations. `Configure`'s job is to wire up the sources, not act as a source itself – Panagiotis Kanavos Mar 04 '21 at 13:26
  • Don't use `ContinueWith` to add continuations, use `await` to add continuations to methods. It makes it much easier to write correct code. – Servy Mar 04 '21 at 13:28
  • @PanagiotisKanavos I agree with you, but unfortunately what I've done seems to be the common way - when detecting changes to appsettings.json files we must debounce multiple change detections per file write (its a failing of the framework), and that is specified on app start in Startup.cs – lonix Mar 04 '21 at 13:30
  • 1
    @lonix the common way is to use the file config provider and let *it* handle changes. There shouldn't be any reason to handle multiple changes, unless `appsettings` is used in an unusual way - rewriting it to modify single entries for example or worse, having the application itself modify the file. If you want to change settings at runtime, use a different source, eg an in-memory source – Panagiotis Kanavos Mar 04 '21 at 13:32
  • @PanagiotisKanavos Its actually a well known [problem](https://github.com/dotnet/aspnetcore/issues/2542) that goes back years. And the way I've done it seems to be the norm from both the repo and here on SO. But I'll take what you said into account, thanks. – lonix Mar 04 '21 at 15:01
  • @PanagiotisKanavos I took your comments into account and settled on a [hosted service](https://stackoverflow.com/a/66491450/9971404) which is async, thanks for putting me on to the right path. I answered the question as it stand anyways. – lonix Mar 05 '21 at 11:11
  • The real issue isn't what that issue describes, which is why it got closed. When you copy a file the Windows will emit one notificatiioin for file creation and one or more for file modification. There's no `file closed` event so there's no way to know when a file finished copying. If an application is smart and allocates all the necessary space *before* writing any changes to the file, you may only get 1 file change event, even for large files. – Panagiotis Kanavos Mar 05 '21 at 11:48
  • 1
    @lonix in fact, the modifying application may still be writing to the target when it gets a change notification, leading to [file access exceptions](https://stackoverflow.com/questions/47330439/filesystemwatcher-cannot-access-files-because-used-in-other-process) when you try to read a modified file. The only solution is to "debounce" the change events, and respond to them only after enough idle time has passed. Libraries like ReactiveX make this a lot easier. Without Rx, you could have each event reset a Timer whose handler takes care of the notifications. – Panagiotis Kanavos Mar 05 '21 at 11:53
  • @lonix that's why OneDrive, DropBox and GDrive don't start syncing immediately when you copy files into their folder. – Panagiotis Kanavos Mar 05 '21 at 11:55
  • @PanagiotisKanavos Yep that's exactly what I did, a [debouncer](https://stackoverflow.com/a/66491450/9971404) approach. Lol is there anything that you don't know about dotnet??? Enjoy your weekend! :) – lonix Mar 05 '21 at 11:56
  • 1
    @lonix I built a file sync tool similar to Dropbox back in 2010, that was used to sync large VHD files. It took me a while to realize why I was getting so few change notifications when someone copied a VHD into the sync folder. Explorer is smart enough to preallocate the entire file. After that, change events are raised only when the copy buffer is flushed to disk. Windows changes the buffer sizes dynamically, so what I observed when copying lots of small files was different from what the customer observed when copying a 4GB Game of Thrones episode "for test purposes" – Panagiotis Kanavos Mar 05 '21 at 12:10
  • @PanagiotisKanavos Ha yes always use real world data for test purposes. On linux/macos I think framework relies on inotify feature instead (which raises notifications after file writes complete), though I could be mistaken as I haven't tested it with "real world data". A debounce of 2-3s seems to work for me on linux in production. – lonix Mar 05 '21 at 12:22

1 Answers1

1

According to this, ASP.NET Core does not have a SynchronizationContext.

So according to comment above by @MatthewWatson, it's fine to use TaskScheduler.Default.

lonix
  • 14,255
  • 23
  • 85
  • 176