Dapper icon indicating copy to clipboard operation
Dapper copied to clipboard

Dapper cancelling only ExecuteReaderAsync not Reader.ReadAsync with MultiMap

Open SelvinPL opened this issue 8 years ago • 2 comments

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 avatar Jul 11 '16 13:07 SelvinPL

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 avatar Jul 12 '16 10:07 SelvinPL

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 ...

SelvinPL avatar Oct 19 '23 22:10 SelvinPL