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

Dapper cancelling only ExecuteReaderAsync not Reader.ReadAsync with MultiMap #555

Open
SelvinPL opened this issue Jul 11, 2016 · 2 comments
Labels
v3.0 Changes awaiting the next breaking release
Milestone

Comments

@SelvinPL
Copy link

SelvinPL commented Jul 11, 2016

Dapper will fail with such Test

        async Task<IEnumerable<Product>> Test()
        {
            CancellationTokenSource cancel = new CancellationTokenSource();
                try
                {
                    const string sql = @"select 1 as id, 'abc' as name, 2 as id, 'def' as name
union all select 1 as id, 'abc' as name, 2 as id, 'def' as name";
                    var productQuery = await connection.QueryAsync<Product, Category, Product>(new CommandDefinition(sql, cancellationToken: cancel.Token), (p, c) => {
                        p.Category = c;
                        //we cancel it in the middle of reading
                        cancel.Cancel();
                        return p;
                    }).ConfigureAwait(false);
                    return productQuery;
                }
                catch (Exception agg)
                {
                    //should be SqlException with message canceled
                }
            return null;
        }

        public async Task TestCancelInReaderReadAsync()
        {
            var res = await Test();
            res.IsEqualTo(null);
        }

solution:
replace all code like(in SQlMapper.Async.cs):

using (var cmd = (DbCommand)command.SetupCommand(cnn, info.ParamReader))
{
/*...*/
}

with

using (var cmd = (DbCommand)command.SetupCommand(cnn, info.ParamReader))
{
     using (var reg = command.CancellationToken.Register(() => cmd.Cancel()))
     {
/*...*/
     }
}

solution is simillar to this answer on SO but he is doing it at wrong place (and not dissposing the registration) ...

@SelvinPL
Copy link
Author

SelvinPL commented Jul 12, 2016

I do some more investigation ... In fact it is because QueryAsync with multimap is using MultiMapImpl which doesn not use reader.ReadAsync() but reader.Read()...
since only MultiMap maper is affected the quick fix is

        private static async Task<IEnumerable<TReturn>> MultiMapAsync<TFirst, TSecond, TThird, TFourth, TFifth, TSixth, TSeventh, TReturn>(this IDbConnection cnn, CommandDefinition command, Delegate map, string splitOn)
        {
            object param = command.Parameters;
            var identity = new Identity(command.CommandText, command.CommandType, cnn, typeof(TFirst), param?.GetType(), new[] { typeof(TFirst), typeof(TSecond), typeof(TThird), typeof(TFourth), typeof(TFifth), typeof(TSixth), typeof(TSeventh) });
            var info = GetCacheInfo(identity, param, command.AddToCache);
            bool wasClosed = cnn.State == ConnectionState.Closed;
            try
            {
                if (wasClosed) await ((DbConnection)cnn).OpenAsync(command.CancellationToken).ConfigureAwait(false);
                using (var cmd = (DbCommand)command.SetupCommand(cnn, info.ParamReader))
                //TODO: quickfix add this line to allow cancellation
                using (var reg = command.CancellationToken.CanBeCanceled ? command.CancellationToken.Register(() => cmd.Cancel()) : (CancellationTokenRegistration?)null)
                using (var reader = await ExecuteReaderWithFlagsFallbackAsync(cmd, wasClosed, CommandBehavior.SequentialAccess | CommandBehavior.SingleResult, command.CancellationToken).ConfigureAwait(false))
                {
                    if (!command.Buffered) wasClosed = false; // handing back open reader; rely on command-behavior
                    //TODO: ... or change this in more like QueryAsync implementation if (command.Buffered) ...
                    var results = MultiMapImpl<TFirst, TSecond, TThird, TFourth, TFifth, TSixth, TSeventh, TReturn>(null, CommandDefinition.ForCallback(command.Parameters), map, splitOn, reader, identity, true);
                    return command.Buffered ? results.ToList() : results;
                }
            } finally
            {
                if (wasClosed) cnn.Close();
            }
        }

EDIT no, it will not works since internally MultiMapImpl use new ownedCommand ...
EDIT2 yes, it will but ... there are 2 different MultiMapAsync so you have to do in both

@SelvinPL SelvinPL changed the title Dapper only cancelling only ExecuteReaderAsync not Reader.ReadAsync Dapper only cancelling only ExecuteReaderAsync not Reader.ReadAsync with MultiMap Jul 12, 2016
SelvinPL added a commit to SelvinPL/dapper-dot-net that referenced this issue Jul 12, 2016
Registering CancellationToken with DbCommand.Cancel will allow
cancellation even with synchrouns DbReader.Read
@SelvinPL SelvinPL changed the title Dapper only cancelling only ExecuteReaderAsync not Reader.ReadAsync with MultiMap Dapper cancelling only ExecuteReaderAsync not Reader.ReadAsync with MultiMap Aug 8, 2016
@NickCraver NickCraver added the v3.0 Changes awaiting the next breaking release label Jan 28, 2017
@NickCraver NickCraver modified the milestone: v2.0 Jan 30, 2017
@SelvinPL
Copy link
Author

Any chances for fixing this?
I'm using Dapper in destop UI app... There is a filter where user can type something then I execute the query with given filter...
If user type there something more I would like cancel query and do new query with new filter - I don't wana "download" unnecesry data from the server
I've stuck with 1.50.x with my modification ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v3.0 Changes awaiting the next breaking release
Projects
None yet
Development

No branches or pull requests

2 participants