3

Closed

Check cancellation token in ForEachAsync [FixedIn6.1.0-alpha1] [AffectedLastRTM]

description

We always delegate the cancellation token check to the provider as it is much more likely that a database operation is being executed when the cancellation is requested.

However it appears that SqlDataReader doesn't check the cancellation token after the first ReadAsync. Also we have more chances to check the cancellation token between database operations during ForEachAsync execution. So if this doesn't impact performance too much we should check cancellation token in ForEachAsync.
Closed Jan 21 at 12:23 AM by jemartti
Verified as fixed along with #1516.

comments

divega wrote Aug 6, 2013 at 6:03 PM

Clearing up the release field after talking to Andriy so that we can triage this again.

divega wrote Aug 7, 2013 at 4:55 PM

We now buffer query results by default, so all the calls to ReadAsync on the underlying data reader will happen before we start materializing results in ForEachAsync. We should consider flowing the cancellation token all the way down to the buffered data reader and check for cancellation requests there (instead of on ForEachAsync), at least for buffered queries.

ocdi wrote Sep 12, 2013 at 4:32 AM

I've been digging into the EF code for the first time and discovered this bit of code in LazyAsyncEnumerator.cs:
    public async Task<bool> MoveNextAsync(CancellationToken cancellationToken)
    {
        var enumerator = await GetEnumeratorAsync(CancellationToken.None).ConfigureAwait(continueOnCapturedContext: false);
        return await enumerator.MoveNextAsync(cancellationToken).ConfigureAwait(continueOnCapturedContext: false);
    }
I am guessing this is what you are referring to in the buffered data reader bit. To me passing cancellationToken into the GetEnumeratorAsync will be better than passing in None, since that appears to be where results are materialised.

Will that break anything significantly changing that parameter here?

In my case it appears to fix the issue.

AndriySvyryd wrote Nov 18, 2013 at 7:34 PM

Good catch ocdi!

moozzyk wrote Dec 5, 2013 at 8:06 PM

Fixed in changeset# d1628ac

Upgrade to Avoid 100% Cancellation Fee - Enabling cancellation in ForEachAsync (Fixes #1470 - https://entityframework.codeplex.com/workitem/1470)

BriceLambson wrote Jan 20 at 7:18 PM

Fixed in changeset d1628ac73594eca176d2701b7e10ede8f84f1eb0