Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why do async LINQ operations ignore synchronization context? #1582

Open
srogovtsev opened this issue Jul 26, 2021 · 7 comments
Open

Why do async LINQ operations ignore synchronization context? #1582

srogovtsev opened this issue Jul 26, 2021 · 7 comments

Comments

@srogovtsev
Copy link

The defining context of my problem is that we're trying to use IAsyncEnumerable and System.Linq.Async within vanilla ASP.NET, which, unlike Core, has and makes heavy usage of synchronization context.

While trying to use some LINQ operations (such as Select) we have unexpectedly found out that the selector functions (and the predicates and Where, and, basically everything) is not guaranteed to run on captured synchronization context. This behavior is unexpected for me, especially given the following quote from Stephen Toub:

Consider, for example, an asynchronous version of LINQ’s Where method, e.g. public static async IAsyncEnumerable<T> WhereAsync(this IAsyncEnumerable<T> source, Func<T, bool> predicate). Does predicate here need to be invoked back on the original SynchronizationContext of the caller? That’s up to the implementation of WhereAsync to decide, and it’s a reason it may choose not to use ConfigureAwait(false).

which basically states that unless we specifically ask for opposite, we should expect the predicates and other callbacks to run on our synchronization context.

Here's a small piece of code to show what we mean:

using (var conn = new SqlConnection(connString))
{
    conn.Open();
    using (var cmd = conn.CreateCommand())
    {
        cmd.CommandText = commandText;

        if (HttpContext.Current == null)
            //this is just a sanity check
            throw new InvalidOperationException();

        var cnt = await Read(cmd).CountAsync(); //a couple of hundred thousand records
        if (HttpContext.Current == null)
            //this works just fine - we're back after the `await`
            throw new InvalidOperationException();

        var data = await Read(cmd).Select(t => 
                HttpContext.Current == null //we'll treat "context lost" as error below
            ).ToArrayAsync();
        if (HttpContext.Current == null)
            //this, too, works just fine - we're again back after the `await`
            throw new InvalidOperationException();

        if (data.Any())
            //this fails: first couple of hundreds invocations are on the context, then they leave the context
            throw new InvalidOperationException();
    }
}

where Read returns a simplest possible implementation of IAsyncEnumerable<T> and IAsyncEnumerator<T> over SqlDataReader.

Of course, the code easily supports our observations: as far as we were able to find, every operation in System.Linq.Async has ConfigureAwait(false).

await foreach (var element in source.WithCancellation(cancellationToken).ConfigureAwait(false))
{
checked
{
index++;
}
yield return selector(element, index);
}

Of course, this looks like a deliberate design decision, so we would like to understand

  1. its rationale
  2. whether we miss something crucial here
  3. whether there are any ways of circumventing this behavior, as we are in dire need of having operations over IAsyncEnumerable working on captured context
@quinmars
Copy link
Contributor

Ideally, a LINQ query does not produce nor rely on side effects, i.e., the operators only work on the data that flow through them and constant data that is captured by the lamda. Whenever this simple rule is not followed, one should be carefull. In the Select operator, ConfigureAwait(false) does not provoke to leave the synchronisation context, it just does not take care to get back on it. It's actually the Read operator that leaves the synchronisation context.

In traditional LINQ the general advice is: "if you have side effects, use foreach". This should work here, too. Alternatively the following method could help you to bring your pipeline back on your synchronisation context:

public static async IAsyncEnumerable<T> OnCurrentSynchronizationContext<T>(this IAsyncEnumerable<T> enumerable)
{
    await foreach (var item in enumerable)
    {
        yield return item;
    }
}

And than:

var data = await Read(cmd)
    .OnCurrentSynchronizationContext()
    .Select(t => HttpContext.Current == null)
    .ToArrayAsync();

@srogovtsev
Copy link
Author

We've been trying to avoid await foreach, as we're still on .Net Framework, which technically does not support C# 8 (if I am not mistaken, of course).

@quinmars
Copy link
Contributor

I feel your pain. Personally, I stopped arguing about C#8 and .NET Framework, and just changed (some of) my projects manually. I think, using IAsyncEnumerable without language support, doesn't make much sense. Your are forced to use ToAsyncArray all over the place, which does in some ways defeats the purpose of the async enumerables.

@julealgon
Copy link

We've been trying to avoid await foreach, as we're still on .Net Framework, which technically does not support C# 8 (if I am not mistaken, of course).

Can't you just reference this package @srogovtsev ? You should be able to use stuff like await foreach by manually changing the LangVersion property on your project and referencing the BCL package:
https://www.nuget.org/packages/Microsoft.Bcl.AsyncInterfaces

As long as the whole team (and build server) uses a modern version of Visual Studio to build your application, it should work fine.

@srogovtsev
Copy link
Author

Yes, this is exactly the shady approach we've been trying to avoid. I say shady because it is not under official support (although it seems to work fins, as you suggest).

@julealgon
Copy link

I wouldn't worry too much about that @srogovtsev . Changing the language version for an entire solution is fairly trivial using Directory.Build.props and it works pretty great from my experience. I have now gone through at least 3 medium-sized projects (100+ project solution) where I targeted C#9 using Net Framework 4.8 projects without any issues.

We are in the process of migrating our current solution from 4.8 to 5 but the benefits some of the new compiler-driven improvements provide are very significant even before that.

@fedeAlterio
Copy link

fedeAlterio commented Apr 10, 2023

@quinmars you said that LINQ queries should not rely on side effects. I think that this is an utopic scenario on UI frameworks. Consider the code below.

  • The ViewModel should know nothing about how the UserService is implemented. So it has no idea that it shows a popup to the user.
  • It's correct for the UserService to assume that the methods are called on the UI Thread.

that's why in my opinion SelectAwait should not be implemented using ConfigureAwait(false) by default. And that's also the reason why await foreach-ing by default captures the caller context.

Of course one solution is to change the code below in order to avoidi LINQ. But at this point I can't see any real world scenario for use it. I think it would be too easy to introduce bugs, or synchronization issues.

public record User(string Name);

public interface IPopupNotification
{
    Task ShowMessage(string message);
}
public interface IUserService
{
    Task<List<User>> GetUsers();
}

public class UserService : IUserService
{
    private readonly IPopupNotification _popupNotification;

    public async Task<List<User>> GetUsers()
    {
        try
        {
            return await WhateverFromApi();
        }
        catch (Exception)
        {
            await _popupNotification.ShowMessage("Error retreiving users");
            throw;
        }
    }
}

internal class MainWindowViewModel : ObservableObject
{
    CancellationTokenSource _cancellationTokenSource = new();
    IUserService _userService;

    async IAsyncEnumerable<int> Timer()
    {
        var token = _cancellationTokenSource.Token;
        for (var i=0; !token.IsCancellationRequested; i++)
        {
            yield return i;

            await Task.Delay(TimeSpan.FromSeconds(1), token);
        }
    }

    public ObservableCollection<User> Users { get; } = new();


    async Task Poll()
    {
        var usersEnumerable = Timer().SelectAwait(async _ => await _userService.GetUsers());
        await foreach(var users in usersEnumerable)
            foreach(var user in users) 
                Users.Add(user);
    }    
}

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

No branches or pull requests

4 participants