Skip to content

Conversation

Shazwazza
Copy link
Contributor

@Shazwazza Shazwazza commented Nov 18, 2020

Ensure that TaskScheduler.Default is used anywhere that ContinueWith is used and adds more debug logging to MainDom operations.

ContinueWith should always have TaskScheduler.Default explicitly declared. There's a few instances in the codebase where this is not the case. It hasn't caused problems in the past that we know about but it is the safe thing to do. For more reading see https://blog.stephencleary.com/2013/10/continuewith-is-dangerous-too.html

This adds a little bit more debug logging to MainDom. This is for debugging purposes only because there's one known site where MainDom shuts down without the application being shutdown and without any SqlMainDomLock logging taking place which is truly odd.

Testing

  • Configure your site to use SqlMainDomLock
  • Add this logging config to your serilog.config
    <add key="serilog:minimum-level:override:Umbraco.Core.Runtime.MainDom" value="Debug" />
    <add key="serilog:minimum-level:override:Umbraco.Core.Runtime.SqlMainDomLock" value="Debug" />
    
  • Test restarting your running app by bumping the web.config, etc...
  • Check the logs during restart MainDom should show it's being signaled, shutdown and acquired each time like normal

…is used, adds more debug logging to MainDom operations
@bergmania bergmania merged commit 9a1b456 into v8/dev Nov 23, 2020
@bergmania bergmania deleted the v8/bugfix/task-scheduler-maindom-logging branch November 23, 2020 06:43
nul800sebastiaan pushed a commit that referenced this pull request Dec 15, 2020
…m-logging

Ensure that TaskScheduler.Default is used anywhere that ContinueWith is used, adds more debug logging to MainDom operations

(cherry picked from commit 9a1b456)

# Conflicts:
#	src/Umbraco.Core/Runtime/SqlMainDomLock.cs
@nul800sebastiaan
Copy link
Member

Cherry picked for 8.6 and above as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants