Otis wrote:

Doesn't that go against what I described here above? https://www.llblgen.com/tinyforum/Message/Goto/151182

Which part? my point is DbDataReader.ReadAsync and SqlDataReader.ReadAsync throw a TaskCanceledException so any loop using ReadAsync will exit with that exception. If your ReadAsync method used DbDataReader.ReadAsync it would also do the same but because its using DbDataReader.Read it doesn't. You've made a change to abort the loop silently so now it behaves differently to if you were using DbDataReader.ReadAsync.

I described it here https://www.llblgen.com/tinyforum/Message/Goto/151177 (earlier post).

AFAIK, there are different ADO.NET providers which act differently so there's no real consensus what to do. Additionally (described in an even earlier post in this thread) there are various problems with DbDataReader.ReadAsync so we don't want to implement a call to that.

Not sure, but it looks like you want me to choose between the situations described in the post I linked above It behaves differently compared to one ado.net provider of a version that's quite recent. It doesn't behave differently when you compare it to System.Data.SqlClient tho, right? (or I must have done my research wrong in the previous post).

Of course we can make a choice, but we won't make a choice for throwing an exception in a point release as that's a breaking change.

Otis wrote:

Looking into it, SqlDataReader.ReadAsync() doesn't thrown OperationCancelledException() when the operation is cancelled

It doesn't behave differently when you compare it to System.Data.SqlClient tho, right? (or I must have done my research wrong in the previous post).

I'm going with wrong!

My point it SqlDataReader.ReadAsync() does throw a OperationCancelledException() when the operation is cancelled

as does DbDataReader.ReadAsync: (CreatedTaskWithCancellation will result in a TaskCanceledException been thrown) and I suspect a lot for ADO.NET providers don't override this

      virtual public Task<bool> ReadAsync(CancellationToken cancellationToken) {
            if (cancellationToken.IsCancellationRequested) {
                return ADP.CreatedTaskWithCancellation<bool>();
            }
            else {
                try {
                    return Read() ? ADP.TrueTask : ADP.FalseTask;
                }
                catch (Exception e) {
                    return ADP.CreatedTaskWithException<bool>(e);
                }
            }
        }

What makes you think it doesn't?

https://github.com/dotnet/SqlClient/issues/1065 seems to me about also cancelling the query on the DB Server not the above behaviour

From System.Data.SqlClient:

// System.Data.SqlClient.SqlDataReader
using System.Data.Common;
using System.Threading;
using System.Threading.Tasks;

public override Task<bool> ReadAsync(CancellationToken cancellationToken)
{
    Bid.ScopeEnter(out IntPtr hScp, "<sc.SqlDataReader.ReadAsync|API> %d#", ObjectID);
    try
    {
        if (IsClosed)
        {
            return ADP.CreatedTaskWithException<bool>(ADP.ExceptionWithStackTrace(ADP.DataReaderClosed("ReadAsync")));
        }
        if (cancellationToken.IsCancellationRequested)
        {
            return ADP.CreatedTaskWithCancellation<bool>();
        }
        if (_currentTask != null)
        {
            return ADP.CreatedTaskWithException<bool>(ADP.ExceptionWithStackTrace(SQL.PendingBeginXXXExists()));
        }
        bool rowTokenRead = false;

It returns a task, doesn't throw an exception. It doesn't register for cancellation later, Microsoft.Data.SqlClient's latest build does. When I looked at this some time ago when you brought it up, it was vague to me what to do... as both optoins have their merit, so I looked into what SqlClient does (there are 3 variants: netfx, netcore's system.data.sqlclient and microsoft.data.sqlclient) and from what I could determine there was no consensus what to do.

As I've told you before, it was very hard for me to come up with a valid test case which made this show up anyway.

Regardless, I'm not going to change the code in a point release to throw an exception all of a sudden. So this will at the soonest be implemented in a future release, however I again have no clear view of what to do in this case as it seems whatever the ado.net provider thinks is ok is done, whether it's silent quit with a message or throw an exception.

I'm indifferent to whether which one is best, I have no opinion on that. I see your argument that it might be better. But again I couldn't make a test work which showed this properly, so adding this in any form would be difficult. The other problem is that in a point release we don't want breaking changes so it's out of the question this is added for a point release.

What I also struggle with, but I mentioned that before, is ... a use case for this particular exception scenario. When cancel is requested, isn't it known by the caller that the operation is canceled... ? Relying on an exception gives me the suggestion that an exception is used for control flow...

Otis wrote:

        if (cancellationToken.IsCancellationRequested)
        {
            return ADP.CreatedTaskWithCancellation<bool>();
        }

It returns a task, doesn't throw an exception.

Yes and no, the above and the snippet I showed from DbDataReader.ReadAsync

            if (cancellationToken.IsCancellationRequested) {
                return ADP.CreatedTaskWithCancellation<bool>();
            }

indeed return a cancelled task. But then the TaskAwaiter sees that and then throws the exception, see:

TaskAwaiter.ValidateEnd(m_task);

Run this in linqpad (with any version of SqlClient) and you will get a TaskCanceledException

public static class Program
{
    public static async Task Main()
    {
        const string longRunningQuery = @"
with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value))
select *
from TenRows;";

        var connectionString = new Microsoft.Data.SqlClient.SqlConnectionStringBuilder
        {
            DataSource = @"(localdb)\MSSQLLocalDB",
            IntegratedSecurity = true,
            TrustServerCertificate = true,
        }.ToString();

        using (var source = new CancellationTokenSource())
        using (var connection = new Microsoft.Data.SqlClient.SqlConnection(connectionString))
        {
            await connection.OpenAsync(source.Token);

            using (var command = new Microsoft.Data.SqlClient.SqlCommand(longRunningQuery, connection))
            using (var reader = await command.ExecuteReaderAsync(source.Token))

                    while (await reader.ReadAsync(source.Token))
                    {
                        source.Cancel();
                    }

        }
    }
}

With this stack trace showing where the exception was thrown

at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter`1.GetResult()

I ran your code using System.Data.SqlClient on .NET 4.8 and it indeed produced the exception. So my analysis was wrong indeed.

Ok, so I went ahead and added the cancellationToken.ThrowIfCancellationRequested() inside the while(reader.Read()) loop and triggered it with a call to FetchQueryAsync(token, query), however it never worked. I first thought this is because the while(reader.Read()) loop doesn't yield to the caller through the async/await stack, so it blocks the caller till the loop is done, but when I changed the datareader loop to call ReadAsync() and pass in the cancellation token, it still didn't work. Perhaps the query is too quick and the cancel comes afterwards, no idea. (The query returns instantly in SMSS).

So I appended WAITFOR, but that didn't work either. So I passed in a cancellation token which should cancel the operation after 1 second. This worked (with the reader.Read()), however it results in a System.OperationCancelledException, not a System.Threading.Tasks.TaskCancelledException.

What a mess.

Doing the same with a plain sql query using just WAITFOR DELAY 00:01 through ExecuteSQLAsync, threw an ORMQueryExecution exception with the exception wrapped.

To be honest, I have no idea what to do and what the use case is for this particular change as in: what problem is currently unsolveable and needs a solution. If it's a timeout, that already works with the command timeout, if it's cancelling at will, I can't make it work as the code to cancel it never hits...

[Test]
public async Task RetrievalQuery_CancellationTest3()
{
    using(var adapter = new DataAccessAdapter())
    {
        var q = @"with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value))
select *
from TenRows;WAITFOR DELAY '00:01'";
        using(var source = new CancellationTokenSource())
        {
            var result = await adapter.FetchQueryAsync<int>(source.Token, q);
            source.Cancel();
        }
    }
}

the source.Cancel line is never reached. but ... all code uses async/await, the call path isn't running into sync code, even if I change reader.Read() into reader.ReadAsync()...

Hmm... the RetrievalQuery's TagAndExecuteQuery might run it synchronously... checking...

(edit) nope, the command.ExecuteReaderAsync() with the WAITFOR query always blocks, no matter what, also in linqpad in an async Main()...

Yes such a pain we cant use WAITFOR to simulate long running queries. But think I've found an alternative using much the same as the previous but with a count to make it slow to return.

public static class Program
{
    public static async Task Main()
    {
        const string longRunningQuery = @"
with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value)),
    ThousandRows as (select A.Value as A, B.Value as B, C.Value as C from TenRows as A, TenRows as B, TenRows as C)
select count(*)
from ThousandRows as A, ThousandRows as B, ThousandRows as C;";

        var connectionString = new SqlConnectionStringBuilder
        {
            DataSource = @"(localdb)\MSSQLLocalDB",
            IntegratedSecurity = true,
            TrustServerCertificate = true,
        }.ToString();

        using (var source = new CancellationTokenSource(TimeSpan.FromMilliseconds(2)))
        using (var connection = new SqlConnection(connectionString))
        {
            await connection.OpenAsync(source.Token);

            using (var command = new SqlCommand(longRunningQuery, connection))
            using (var reader = await command.ExecuteReaderAsync(source.Token))
                try
                {
                    Console.WriteLine("ExecuteReaderAsync a: " + source.Token.IsCancellationRequested + " " + reader.IsClosed);
                    while (await reader.ReadAsync()) //source.Token - SqlException with no token, TaskCanceledException with it
                    {
                        source.Cancel();
                    }
                }
                catch (Exception e)
                {
                    Console.WriteLine("Cancelled:" + e);
                } 
        }
    }
}

So now we can simulate both scenarios:

a: Query that returns quickly but takes along time to read the rows

b: Query that takes along time to return

FromMilliseconds(2) cancels basically the connection open. (following the stacktrace in my test)

If I remove it:

[Test]
public async Task RetrievalQuery_Cancellation_Async()
{
    const string longRunningQuery = @"
with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value)),
ThousandRows as (select A.Value as A, B.Value as B, C.Value as C from TenRows as A, TenRows as B, TenRows as C)
select count(*)
from ThousandRows as A, ThousandRows as B, ThousandRows as C;";

    using(var adapter = new DataAccessAdapter())
    {
        using(var source = new CancellationTokenSource())
        {
            var results = await adapter.FetchQueryAsync<int>(source.Token, longRunningQuery).ConfigureAwait(false);
            source.Cancel();
        }
    }
}

The query always succeeds, even if I throw an exception in the reader loop if the token is cancelled, or if I do while(reader.ReadAsync(token)) instead, it always succeeds, the line source.Cancel() is never called before the query ends.

I'm at a loss why, but it's the same with vanilla ADO.NET:

public static async Task Main(string[] args)
{
    const string longRunningQuery = @"
with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value)),
ThousandRows as (select A.Value as A, B.Value as B, C.Value as C from TenRows as A, TenRows as B, TenRows as C)
select count(*)
from ThousandRows as A, ThousandRows as B, ThousandRows as C;";

    var connectionString = new SqlConnectionStringBuilder
                           {
                               DataSource = @"(localdb)\MSSQLLocalDB",
                               IntegratedSecurity = true,
                               TrustServerCertificate = true,
                           }.ToString();

    using (var source = new CancellationTokenSource())
    using (var connection = new SqlConnection(connectionString))
    {
        await connection.OpenAsync(source.Token);

        using (var command = new SqlCommand(longRunningQuery, connection))
        using (var reader = await command.ExecuteReaderAsync(source.Token))
            try
            {
                Console.WriteLine("ExecuteReaderAsync a: " + source.Token.IsCancellationRequested + " " + reader.IsClosed);
                await ReadRows(reader, source.Token).ConfigureAwait(false);
                source.Cancel();
            }
            catch (Exception e)
            {
                Console.WriteLine("Cancelled:" + e);
            } 
    }
}


public static async Task ReadRows(SqlDataReader reader, CancellationToken token)
{
    while (await reader.ReadAsync(token)) //source.Token - SqlException with no token, TaskCanceledException with it
    {
    }
}

it's never cancelled.

So what's the use case, what problem am I solving? I've now spend several hours trying all kinds of scenarios but I have no idea what problem I'm solving

So if I run my test with the current 5.9.3 code:

[Test]
public async Task RetrievalQuery_Cancellation_Async()
{
    const string longRunningQuery = @"
with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value)),
ThousandRows as (select A.Value as A, B.Value as B, C.Value as C from TenRows as A, TenRows as B, TenRows as C)
select count(*)
from ThousandRows as A, ThousandRows as B, ThousandRows as C;";

    using(var adapter = new DataAccessAdapter())
    {
        using(var source = new CancellationTokenSource(TimeSpan.FromSeconds(1)))
        {
            var results = await adapter.FetchQueryAsync<int>(source.Token, longRunningQuery).ConfigureAwait(false);
        }
    }
}

And set the cancellation to 1 second, I get a nice exception that the operation was cancelled

System.Data.SqlClient.SqlException : A severe error occurred on the current command.  The results, if any, should be discarded.
Operation cancelled by user.
Data:
  HelpLink.ProdName: Microsoft SQL Server
  HelpLink.ProdVer: 11.00.7001
  HelpLink.EvtSrc: MSSQLServer
  HelpLink.EvtID: 0
  HelpLink.BaseHelpUrl: http://go.microsoft.com/fwlink
  HelpLink.LinkId: 20476

   at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlDataReader.TryHasMoreRows(Boolean& moreRows)
   at System.Data.SqlClient.SqlDataReader.TryReadInternal(Boolean setTimeout, Boolean& more)
   at System.Data.SqlClient.SqlDataReader.Read()
   at SD.Tools.OrmProfiler.Interceptor.ProfilerDbDataReader.Read()
   at SD.LLBLGen.Pro.ORMSupportClasses.ProjectionUtils.FetchProjectionFromReaderAsync[T](List`1 destination, IDataReader dataSource, IRetrievalQuery queryExecuted, CancellationToken cancellationToken, Boolean performImplicitTypeConversions) in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\Projection\ProjectionUtils.cs:line 291
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.<FetchProjectionAsync>d__200`1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\AdapterSpecific\DataAccessAdapterCore.cs:line 4342
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.<FetchQueryAsync>d__187`1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\AdapterSpecific\DataAccessAdapterCore.cs:line 3973
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterBase.<ExecuteWithActiveRecoveryStrategyAsync>d__49`1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\AdapterSpecific\DataAccessAdapterBase.cs:line 969
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at Unittests.TestLibrary.SqlServerTests.Adapter.Async.AsyncRawSQLTests_Names.<RetrievalQuery_Cancellation_Async>d__1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\UnitTests\LLBLGen Pro\LowLevelAPI\NUnitTests\Adapter\AsyncPersistenceTests\AsyncRawSQLTests_Names.cs:line 65
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at NUnit.Framework.Internal.TaskAwaitAdapter.GenericAdapter`1.BlockUntilCompleted()
   at NUnit.Framework.Internal.AsyncToSyncAdapter.Await(Func`1 invoke)
   at NUnit.Framework.Internal.Commands.TestMethodCommand.Execute(TestExecutionContext context)
   at NUnit.Framework.Internal.Execution.SimpleWorkItem.PerformWork()

If I do a cancellationToken.ThrowIfCancellationRequested() inside the while(reader.Read()) loop, it still throws a SqlException, so that line is likely not hit properly (as the reader.Read() checks are earlier in this particular situation). The only way to 'fix' that (if that's the problem) is to use ReadAsync instead of Read but that has serious side effects Microsoft hasn't all solved yet so we're not going to do that.

So not sure what changes I have to make to fix a problem I have no idea we currently have...

Otis wrote:

If I do a cancellationToken.ThrowIfCancellationRequested() inside the while(reader.Read()) loop, it still throws a SqlException, so that line is likely not hit properly (as the reader.Read() checks are earlier in this particular situation). The only way to 'fix' that (if that's the problem) is to use ReadAsync instead of Read but that has serious side effects Microsoft hasn't all solved yet so we're not going to do that.

So not sure what changes I have to make to fix a problem I have no idea we currently have...

I don't have time for a full answer but as I suggested before call cancellationToken.ThrowIfCancellationRequested() before the read then it will behave like ReadAsync. If it were me, I would replace the read calls with something like

    public static bool Read(this IDataReader dataReader, CancellationToken cancellationToken)
    {
      cancellationToken.ThrowIfCancellationRequested();
      return dataReader.Read();
    }

I tried that But as I described above, the reader.Read() call already picks up the cancelled token and throws a SqlException, the ThrowIfCancellationRequested call wasn't hit in my tests, the exception originated from the Read() call itself.

Otis wrote:

I tried that But as I described above, the reader.Read() call already picks up the cancelled token and throws a SqlException, the ThrowIfCancellationRequested call wasn't hit in my tests, the exception originated from the Read() call itself.

Not if you call ThrowIfCancellationRequested before you call read,

What do you mean reader.Read() call picks up the cancelled token? if the command has been cancelled by the token Read will throw a SqlException.

Consider this query that both takes about second to return and about 5 seconds to read all the rows. By adjusting TimeSpan.FromSeconds you can make the token cancel during ExecuteReaderAsync or during reading the rows, in both cases it throws an OperationCancelledException. Which is what I think should happen, same outcome no matter when it gets cancelled.

public static class Program
{
    public static async Task Main()
    {
        const string longRunningQuery = @"
DECLARE @times int;
with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value)),
    ThousandRows as (select A.Value as A, B.Value as B, C.Value as C from TenRows as A, TenRows as B, TenRows as C)

select count(*)
from ThousandRows as A, ThousandRows as B, ThousandRows as C;

with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value)),
    ThousandRows as (select A.Value as A, B.Value as B, C.Value as C from TenRows as A, TenRows as B, TenRows as C)
select *
from ThousandRows as A, ThousandRows as B;
";
        var connectionString = new SqlConnectionStringBuilder
        {
            DataSource = @"(localdb)\MSSQLLocalDB",
            IntegratedSecurity = true,
            TrustServerCertificate = true,
        }.ToString();

        using (var source = new CancellationTokenSource(TimeSpan.FromSeconds(1))) //1 sec cancelled during ExecuteReaderAsync, 2 cancelled during Read
        using (var connection = new SqlConnection(connectionString))
        {
            await connection.OpenAsync(source.Token);
            Console.WriteLine("OpenAsync: " + source.Token.IsCancellationRequested + " " + connection.State);
            using (var command = new SqlCommand(longRunningQuery, connection))
            using (var reader = await command.ExecuteReaderAsync(source.Token))
                try
                {
                    Console.WriteLine("ExecuteReaderAsync: " + source.Token.IsCancellationRequested + " " + reader.IsClosed);
                    var rowIndex = 0;
                    while (reader.Read(source.Token)) 
                    {
                        rowIndex++;
                        Console.WriteLine("Row:" + rowIndex + source.Token.IsCancellationRequested);
                    }
                    await reader.NextResultAsync(source.Token);
                    Console.WriteLine("NextResultAsync: " + source.Token.IsCancellationRequested + " " + reader.IsClosed);
                    while (reader.Read(source.Token))
                    {
                        rowIndex++;
                        Console.WriteLine("Row:" + rowIndex + source.Token.IsCancellationRequested);
                    }
                }
                catch (Exception e)
                {
                    Console.WriteLine("Cancelled:" + e);
                } // If you pause the debugger, it highlights this line
        }
    }

    public static bool Read(this IDataReader dataReader, CancellationToken cancellationToken)
    {
        cancellationToken.ThrowIfCancellationRequested();
        return dataReader.Read();
    }
}

TomDog wrote:

Otis wrote:

I tried that But as I described above, the reader.Read() call already picks up the cancelled token and throws a SqlException, the ThrowIfCancellationRequested call wasn't hit in my tests, the exception originated from the Read() call itself.

Not if you call ThrowIfCancellationRequested before you call read,

It's a race condition, the Read() might be completed before the cancellation token is cancelled, and then in the processing (the loop code around Read()) a check is done and it'll throw if the cancellation token is cancelled. So that's 1 scenario. The other is that the cancellation token is cancelled when the Read() call is happening, and it then throws a different exception.

What do you mean reader.Read() call picks up the cancelled token? if the command has been cancelled by the token Read will throw a SqlException.

Yes that's what I meant: it'll throw a SqlException. I couldn't make it throw an OperationCancelled exception.

Consider this query that both takes about second to return and about 5 seconds to read all the rows. By adjusting TimeSpan.FromSeconds you can make the token cancel during ExecuteReaderAsync or during reading the rows, in both cases it throws an OperationCancelledException. Which is what I think should happen, same outcome no matter when it gets cancelled.

This still can lead to a SqlException, if the token is cancelled right after cancellationToken.ThrowIfCancellationRequested(); returns, so inside the Read() call.

But we can debate this forever. What I want is a clear problem description what's wrong so we can address that to see if it needs a solution and if so, which one. If it's: "The engine has to throw an OperationCancelledException when the operation was cancelled", this seems to be doable only when we refactor the code to use reader.ReadAsync(). that's not going to happen. If it's "I want to timeout the command properly", please use the commandtimeout on the adapter.

Atm the engine will throw a SqlException when the operation is cancelled. it's inconvenient but it's a price I'm willing to pay for the choice of using ReadAsync() which has its own problems and we then have to deal with those.

Otis wrote:

But we can debate this forever.

I don't feel I'm debating anything just trying to establish some basic facts, I don't see why we shouldn't be able to agree on those since we have the same examples.

Speaking of which

Otis wrote:

This still can lead to a SqlException, if the token is cancelled right after cancellationToken.ThrowIfCancellationRequested(); returns, so inside the Read() call.

I have not observed that, consider this Pseudo Code

    1. ExecuteReaderAsync
    2. bool readResult = true;
    3. Do until readResult is false
        a. ThrowIfCancellationRequested
        b. readResult = reader.Read();
        c. If (result) do processing

If I cancel the token after 3a the read is unaffected.

Otis wrote:

What I want is a clear problem description what's wrong so we can address that to see if it needs a solution and if so, which one.

What want is that 'The engine to throw an exception when the operation is cancelled and behave the same no mater when during the operation the cancel occurs, ideally the exception is an OperationCancelledException'

Currently if the operation is cancelled before the first read it throws a SqlException and if its cancelled after the first read it doesn't throw any exception.

To put the problem briefly: its inconsistent

TomDog wrote:

Otis wrote:

But we can debate this forever.

I don't feel I'm debating anything just trying to establish some basic facts, I don't see why we shouldn't be able to agree on those since we have the same examples.

My remark was more about: we can debate about native ADO.NET examples and what they do but without a clear problem description we can't decide 1) what the best solution for that will be and 2) IF we even will fix it.

Speaking of which

Otis wrote:

This still can lead to a SqlException, if the token is cancelled right after cancellationToken.ThrowIfCancellationRequested(); returns, so inside the Read() call.

I have not observed that, consider this Pseudo Code

    1. ExecuteReaderAsync
    2. bool readResult = true;
    3. Do until readResult is false
        a. ThrowIfCancellationRequested
        b. readResult = reader.Read();
        c. If (result) do processing

If I cancel the token after 3a the read is unaffected.

What do you mean with 'unaffected' ? It's the same as when I do:

    1. ExecuteReaderAsync
    2. bool readResult = true;
    3. Do until readResult is false
        a. readResult = reader.Read();
        b. ThrowIfCancellationRequested
        c. If (result) do processing

and cancel right before 3a above. The code above is what I had and I couldn't make it work, simply because the cancellation occurred most of the time inside the read operation as that took more time than the rest of the loop combined.

Otis wrote:

What I want is a clear problem description what's wrong so we can address that to see if it needs a solution and if so, which one.

What want is that 'The engine to throw an exception when the operation is cancelled and behave the same no mater when during the operation the cancel occurs, ideally the exception is an OperationCancelledException'

Currently if the operation is cancelled before the first read it throws a SqlException and if its cancelled after the first read it doesn't throw any exception. To put the problem briefly: its inconsistent

That's not what I see, see my post above: https://www.llblgen.com/tinyforum/Message/Goto/151278, it throws a SqlException inside the Read() call. Hence my puzzlement about what it is we need to chance

Otis wrote:

What do you mean with 'unaffected' ? It's the same as when I do:

    1. ExecuteReaderAsync
    2. bool readResult = true;
    3. Do until readResult is false
        a. readResult = reader.Read();
        b. ThrowIfCancellationRequested
        c. If (result) do processing

and cancel right before 3a above. The code above is what I had and I couldn't make it work, simply because the cancellation occurred most of the time inside the read operation as that took more time than the rest of the loop combined.

This is the crux of our difference, in your version of the Pseudo Code how do you know the cancellation occurred in the read? I believe the cancellation occurs during the ExecuteReaderAsync execution and because no exception is thrown the read gets executed and throws a SqlExecption.

We can test that by putting a ThrowIfCancellationRequested call between the ExecuteReaderAsync and the read (as in my version), if it throws you know the cancellation occurred in ExecuteReaderAsync.

Another thing worth doing is looking at the _pendingCancel field on the command before the read.

                var commandExposed = Exposed.From(command);
                Console.WriteLine("After ExecuteReaderAsync: " + source.Token.IsCancellationRequested + " " + commandExposed._pendingCancel);

If _pendingCancel is true Read will throw SqlExecption

TomDog wrote:

Otis wrote:

What do you mean with 'unaffected' ? It's the same as when I do:

    1. ExecuteReaderAsync
    2. bool readResult = true;
    3. Do until readResult is false
        a. readResult = reader.Read();
        b. ThrowIfCancellationRequested
        c. If (result) do processing

and cancel right before 3a above. The code above is what I had and I couldn't make it work, simply because the cancellation occurred most of the time inside the read operation as that took more time than the rest of the loop combined.

This is the crux of our difference, in your version of the Pseudo Code how do you know the cancellation occurred in the read? I believe the cancellation occurs during the ExecuteReaderAsync execution and because no exception is thrown the read gets executed and throws a SqlExecption.

Why would there be no exception thrown in ExecuteReaderAsync if the cancellation occurs during the ExecuteReaderAsync call? Isn't it safe to assume it's going to be thrown there if it's cancelled there?

If I look at the stacktrace in my post above:

at System.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at System.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at System.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
   at System.Data.SqlClient.SqlDataReader.TryHasMoreRows(Boolean& moreRows)
   at System.Data.SqlClient.SqlDataReader.TryReadInternal(Boolean setTimeout, Boolean& more)
   at System.Data.SqlClient.SqlDataReader.Read()
   at SD.Tools.OrmProfiler.Interceptor.ProfilerDbDataReader.Read()
   at SD.LLBLGen.Pro.ORMSupportClasses.ProjectionUtils.FetchProjectionFromReaderAsync[T](List`1 destination, IDataReader dataSource, IRetrievalQuery queryExecuted, CancellationToken cancellationToken, Boolean performImplicitTypeConversions) in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\Projection\ProjectionUtils.cs:line 291
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.<FetchProjectionAsync>d__200`1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\AdapterSpecific\DataAccessAdapterCore.cs:line 4342

The cancellation happens during the Read() call and therefore I get a sqlexception. Like I said, I couldn't get the code to throw an OperationCancelledException, it always threw a SqlException. But ... it did throw an exception. So it's not inconsistent as you said, which might be due to different ADO.NET providers or even versions, who knows (as there is inconsistency there).

Anyway, why would this be bad? You get an exception, operation is cancelled! Isn't that what you wanted?

Ok, a couple of things:

1) when I change the reader.Read() into reader.ReadAsync() it still does a Read() when I execute it with the profiler enabled on .net fx (this needs to be fixed) (edit) this turned out to be a fluke with referenced assemblies... 2.0.5 works fine.

2) we'll switch to reader.ReadAsync() in the runtime in v5.10 (which will go into beta in January 2023). It will have problems in some scenarios but that's the problem of the ADO.NET provider with the actual issue. We can't otherwise make this work like it is expected (i.e. throwing a TaskCancelledException.)

Otis wrote:

Why would there be no exception thrown in ExecuteReaderAsync if the cancellation occurs during the ExecuteReaderAsync call? Isn't it safe to assume it's going to be thrown there if it's cancelled there?

No its not as we discussed here https://www.llblgen.com/tinyforum/Thread/27630#151184, it cancels silently(i.e. doesn't throw an exception) unless there is a WAIT in the query.

Otis wrote:

The cancellation happens during the Read() call and therefore I get a sqlexception

Cancellation happens during the ExecuteReaderAsync the exception occurs during the Read because of the state of the command

TomDog wrote:

Otis wrote:

Why would there be no exception thrown in ExecuteReaderAsync if the cancellation occurs during the ExecuteReaderAsync call? Isn't it safe to assume it's going to be thrown there if it's cancelled there?

No its not as we discussed here https://www.llblgen.com/tinyforum/Thread/27630#151184, it cancels silently(i.e. doesn't throw an exception) unless there is a WAIT in the query.

In my query there's no wait, I use this query you gave earlier:

with TenRows as (select Value from (values (1), (2), (3), (4), (5), (6), (7), (8), (9), (10)) as TenRows (Value)),
    ThousandRows as (select A.Value as A, B.Value as B, C.Value as C from TenRows as A, TenRows as B, TenRows as C)
select count(*)
from ThousandRows as A, ThousandRows as B, ThousandRows as C;

However upon close inspection, it returns just 1 row, after 1.7 seconds, so the reader is of course not going to used much indeed, and you're right!

If I change it to fetch 500000 rows using top, it takes 617ms, but timing the cancel to fall into the read loop to produce a sqlexception is difficult, it now always throws a TaskCancelledException, even if I set the timespan to 550ms (total query is 617ms).

System.Threading.Tasks.TaskCanceledException : A task was canceled.
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.<FetchProjectionAsync>d__200`1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\AdapterSpecific\DataAccessAdapterCore.cs:line 4342
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult()
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterCore.<FetchQueryAsync>d__187`1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\AdapterSpecific\DataAccessAdapterCore.cs:line 3973
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at SD.LLBLGen.Pro.ORMSupportClasses.DataAccessAdapterBase.<ExecuteWithActiveRecoveryStrategyAsync>d__49`1.MoveNext() in C:\MyProjects\VS.NET Projects\LLBLGen Pro v5.9\Frameworks\LLBLGen Pro\RuntimeLibraries\ORMSupportClasses\AdapterSpecific\DataAccessAdapterBase.cs:line 969

etc... but... the reader is consumed using Read().

In any case, we've switched it to ReadAsync() in 5.10 to be done with this, as how it is today in 5.9 is 1) hard to test (I can't reliably reproduce what you say in the reader loop) and 2) using ReadAsync is consistent with what should be done with the API anyway.

There's still an issue with the profiler. The ProfilerDbDataReader.Close() does a sync Read() apparently, which triggers a SqlException when the task is cancelled. If I don't use the profiler, I get a TaskCancelledException in 5.10's tests now. This is a bug and we'll try to fix that

(edit) it's an optimization to cancel the command when closing the reader, only on netfx. We'll remove that as it's not helpful

(edit) removed in ORM Profiler 2.0.6