dotnet / SqlClient

Microsoft.Data.SqlClient provides database connectivity to SQL Server for .NET applications.
MIT License
853 stars 286 forks source link

ExecuteNonQueryAsync occur UnobservedTaskException #2104

Open junhun0106 opened 1 year ago

junhun0106 commented 1 year ago

exception log

Microsoft.Data.SqlClient.SqlException (0x80131904): A transport-level error has occurred when receiving results from the server. (provider: TCP Provider, error: 0 - An existing connection was forcibly closed by the remote host.)
---> System.ComponentModel.Win32Exception (10054): An existing connection was forcibly closed by the remote host.
   at Microsoft.Data.SqlClient.SqlConnection.OnError(SqlException exception, Boolean breakConnection, Action`1 wrapCloseInAction)
   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParserStateObject.ThrowExceptionAndWarning(Boolean callerHasConnectionLock, Boolean asyncClose)
   at Microsoft.Data.SqlClient.TdsParserStateObject.ReadSniError(TdsParserStateObject stateObj, UInt32 error)
   at Microsoft.Data.SqlClient.TdsParserStateObject.ReadSni(TaskCompletionSource`1 completion)
   at Microsoft.Data.SqlClient.SqlCommand.BeginExecuteNonQueryInternalReadStage(TaskCompletionSource`1 completion)
   at Microsoft.Data.SqlClient.SqlCommand.BeginExecuteNonQueryInternal(CommandBehavior behavior, AsyncCallback callback, Object stateObject, Int32 timeout, Boolean inRetry, Boolean asyncWrite)
   at Microsoft.Data.SqlClient.SqlCommand.BeginExecuteNonQueryAsync(AsyncCallback callback, Object stateObject)
   at System.Threading.Tasks.TaskFactory`1.FromAsyncImpl(Func`3 beginMethod, Func`2 endFunction, Action`1 endAction, Object state, TaskCreationOptions creationOptions)
   at Microsoft.Data.SqlClient.SqlCommand.InternalExecuteNonQueryAsync(CancellationToken cancellationToken)

explanation

 private IAsyncResult BeginExecuteNonQueryInternal(...)
{
  // ...
  TaskCompletionSource<object> localCompletion = new TaskCompletionSource<object>(stateObject);
  // ...

  try
  {
     // ...
     if (execNQ != null)
     {
       // ...
     }
     else
     {
        // throw exception!!
        BeginExecuteNonQueryInternalReadStage(localCompletion);
     }
  }
  catch(Exception ex)
  {
    // ...
    // not resolved localCompletion Task
    throw;
  }

detail

TdsParserStateObject.ReadSni(...)
{
 // ...
 _networkPacketTaskSource = completion;
 // ...
}

TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, ...)
{
 if (breakConnection)
 {
   // ...

   // report exception to pending async operation
   // before OnConnectionClosed overrides the exception
   // due to connection close notification through references
   TaskCompletionSource<object> taskSource = stateObj._networkPacketTaskSource;
   if (taskSource != null)
   {
      taskSource.TrySetException(ADP.ExceptionWithStackTrace(exception));
   }
 }

   // ...

   // occur throw exception
  _connHandler.OnError(exception, breakConnection, wrapCloseAction);
  // else if (exception.Class >= TdsEnums.MIN_ERROR_CLASS)
  // {
  //    // It is an error, and should be thrown.  Class of TdsEnums.MIN_ERROR_CLASS
  //    // or above is an error, below TdsEnums.MIN_ERROR_CLASS denotes an info message.
  //    throw exception;
  // }
}

Lastly, as I am not a native English speaker, I ask for your understanding if there are any shortcomings in my use of the language. Thank you.

roji commented 1 year ago

@junhun0106 can you please submit a minimal, runnable console program which shows the exception occuring? It's not possible to know from the above what your code is doing, and if it's possibly doing something unsupported.

junhun0106 commented 1 year ago

@roji My execution code is very simple. It was written as follows, and an error occurred while changing the Azure Db during execution. My code provides a retry for failover, but it failed to retry properly because an UnobservedTaskException occurred.

using(var conn = new SqlConnection(...))
{
   // ...
   await cmd.ExcecuteNonQueryAsync(...)
}

Given the context of the code, if breakConnection is true, it operates in the following form. It causes an exception because localCompletion is not handled anywhere.


// setting localCompletion(=stateObj._networkPacketTaskSource)
if(breakConnection)
{ 
  localCompletion.TrySetException(ADP.ExceptionWithStackTrace(exception));
}
// throw exception _connection.OnError
throw exception;

// localCompletion is not handled anywhere
// 
junhun0106 commented 1 year ago

@roji tried to simplify the code I'm using and the library code as much as possible. This is an example where the exception set in localCompletion.TrySetException() falls into an UnobservedTaskException.

internal class Program
{
    class Context
    {
        public readonly TaskCompletionSource source;

        public Context(TaskCompletionSource source)
        {
            this.source = source;
        }
    }

    static async Task ExecuteWithFailover(Func<Task> func)
    {
        try
        {
            await func();
        }
        catch (Exception ex)
        {
            Console.WriteLine($"ExecuteWithFailover - {ex.Message}");
        }
        finally
        {
            Console.WriteLine("complete");
        }
    }

    static async Task ExecuteImplAsync()
    {
        try
        {
            await InternalExecuteNonQueryAsync();
        }
        finally
        {
        }
    }

    static void BeginExecuteNonQueryInternalReadStage(TaskCompletionSource<object> completion)
    {
       // like if (breakConnection)
        completion.SetException(new Exception("somting error"));

        // like _connection.Error(...) thrown
        throw new Exception("somting error");
    }

    static IAsyncResult BeginExecuteNonQueryInternal(AsyncCallback callback)
    {
        TaskCompletionSource<object> localCompletion = new TaskCompletionSource<object>();
        TaskCompletionSource<object> globalCompletion = new TaskCompletionSource<object>();

        try
        {
            BeginExecuteNonQueryInternalReadStage(localCompletion);
        }
        catch
        {
            throw;
        }

        localCompletion.Task.ContinueWith(t =>
        {
            if (t.IsFaulted)
            {
                globalCompletion.TrySetException(t.Exception.InnerException);
            }
            else if (t.IsCanceled)
            {
                globalCompletion.TrySetCanceled();
            }
            else
            {
                globalCompletion.TrySetResult(t.Result);
            }
        }, TaskScheduler.Default);

        globalCompletion.Task.ContinueWith
            (
            static (task, state) =>
            {
                ((AsyncCallback)state)(task);
            },
            state: callback
            );

        return globalCompletion.Task;
    }

    static Task InternalExecuteNonQueryAsync()
    {
        TaskCompletionSource source = new TaskCompletionSource();

        var returnTask = source.Task;

        returnTask = returnTask.ContinueWith(
            static (task, state) =>
            {
                return task;
            },
            state: null)
            .Unwrap();

        var context = new Context(source);

        Task.Factory.FromAsync(
            static (callback, stateObject) => BeginExecuteNonQueryInternal(callback),
            static result =>
            {
                Exception ex = ((Task)result).Exception;
                if (ex != null)
                {
                    Console.WriteLine($"endMethod - {ex.InnerException.Message}");
                    throw ex.InnerException;
                }
                else
                {
                    Console.WriteLine("endMethod");
                }
            },
            state: context)
            .ContinueWith(
            (task, state) =>
            {
                var ctx = (Context)state;
                var s = ctx.source;

                if (task.IsFaulted)
                {
                    s.SetException(task.Exception.InnerException);
                }
                else if (task.IsCanceled)
                {
                    s.SetCanceled();
                }
                else
                {
                    s.SetResult();
                }
            },
            state: context,
            scheduler: TaskScheduler.Default);

        return returnTask;
    }

    private static void Main(string[] args)
    {
        TaskScheduler.UnobservedTaskException += (s, e) =>
        {
            e.SetObserved();

            if (e.Exception.InnerException != null)
            {
                Console.WriteLine("ThreadSchedulerUnobservedException " + e.Exception.InnerException.Message);
            }
            else
            {
                Console.WriteLine("ThreadSchedulerUnobservedException " + e.Exception.Message);
            }
        };

        Task.Run(async () =>
        {
            try
            {
                await ExecuteWithFailover(async () =>
                {
                    await ExecuteImplAsync();
                });
            }
            catch
            {
                // failover retry
            }
        });

        while (true)
        {
            GC.Collect();

            Thread.Sleep(100);
        }
    }
}
roji commented 1 year ago

@junhun0106 I don't see any actual database code (e.g. UnobservedTaskException) in this sample, so this doesn't seem to be related to SqlClient in any way.

I can look into your code, but you're very likely not handling an exception somewhere, and so it's unobserved. This is one of the reasons to avoid the notoriously difficult - and at this point pretty much obsolete - Begin/End APIs with continuiations (ContinueWith).

Is there any reason you're not using modern async/await instead?

junhun0106 commented 1 year ago

@roji

sample code is simply database code. not actual code.

Here is using actual code in callstack BeginExecuteNonQueryInternal

ThrowExceptionAndWarning api called set localCompletion.TrySetException when breakConnection is true

BeginExecuteNonQueryInternalReadStage occurs throw exception in OnError(...) not handled TaskCompletionSource localCompletion

and collected GC occurs TaskScheduler.UnobservedTaskException

I should have provided the link earlier, I apologize for wasting your time.

roji commented 1 year ago

@junhun0106 I'll let a SqlClient developer respond to the above (I don't work on SqlClient), but it's really best if you can submit a minimal, runnable console program using SqlClient, which shows how the SqlClient code you're saying is problematic results in a user-visible bug.

junhun0106 commented 1 year ago

@roji

The sample program simply opens the SqlConnection and executes it. because it actually break the connection to real database. (I used Azure Db)

so, I uploaded a sample where a Task becomes unresolved using similar logic.

Thank you for your advice.

Kaur-Parminder commented 1 year ago

@junhun0106 Can you share screenshot of call stack from your IDE when exception occurred? and generate EventSource logs for further investigation.

junhun0106 commented 1 year ago

@Kaur-Parminder

During development, it's not easy to recreate a situation where we only have logs from the stress test and the process of expanding the database.

However, there are instances where tasks remain Unresolved in the code, and I'm curious if it's really necessary to have a 'call stack' and EventSource.

The exception log is included in the original text.

I apologize for the late response.