V12: Suppress execution context flow when queuing email task

[nikolajlauridsen]

Suppresses the execution context flow when queuing a task for sending notification emails. This is required because otherwise, the execution context would leak into the async thread that won't be joined.

This caused issues when trying to replace the IEmailSender with a different implementation that creates a scope since the execution context was flowed, it gets added to the same scope stack as the other thread, leading to race conditions that would throw errors because a child scope on the other thread wasn't disposed. This should no longer happen.

It looks like there's a lot of changes in the PR, but all I did was wrapping the implementation of NotificationService.Process in a using (ExecutionContext.SuppressFlow()) block.

Testing

Since this is essentially a race condition it's pretty hard to test, but I was able to reproduce the issue somewhat reliably with the following approach (Thanks to the heartcore team for the test setup and heads up 💪):

1. Add a custom IEmailSender that calls UserService.GetByEmail (see below for example)
2. Replace it in ConfigureServices with: services.Replace(ServiceDescriptor.Singleton<IEmailSender, HeadlessEmailSender>());
3. Create a simple document type and content node (doesn't need any content)
4. Set up notifications on publish
5. Restart Umbraco
6. Click Save and publish as the first action

This will throw a System.InvalidOperationException, not all the time, but more often than not.

After checking out this branch the error should no longer occur.

Code snippet:

using Umbraco.Cms.Core.Mail;
using Umbraco.Cms.Core.Models.Email;
using Umbraco.Cms.Core.Services;

namespace Umbraco.Cms.Web.UI;

public class HeadlessEmailSender : IEmailSender
{
    private IUserService _userService;

    public HeadlessEmailSender(IUserService userService)
    {
        _userService = userService;
    }

    public bool CanSendRequiredEmail()
    {
        return true;
    }

    public Task SendAsync(EmailMessage message, string emailType)
    {
        _userService.GetByEmail(message.To.Single()!);
        return Task.CompletedTask;
    }

    public Task SendAsync(EmailMessage message, string emailType, bool enableNotification)
    {
        throw new NotImplementedException();
    }
}

[elit0451]

Looks good and tests out good 🙌

Merging!

[Zero3]

Thanks for the fix guys! 👍

Glad to see that you found a solution and released it already. I was afraid that it was me that was seeing ghosts or messing something up badly somehow, given that my code sometimes worked and sometimes failed miserably.

[nikolajlauridsen]

Yeah it was a bit of an ugly race condition, but thanks a lot for letting us know so we could get it fixed 💪