madelson / DistributedLock

A .NET library for distributed synchronization
MIT License
1.74k stars 182 forks source link

Too many UnobservedTaskException #192

Closed nelsoncvjr-stone closed 3 months ago

nelsoncvjr-stone commented 3 months ago

Application info

Framework: .NET 8.0
Package: DistributedLock.SqlServer 1.0.3

I have the following code:

LockManager

public async ValueTask<LockHandlerToken> TryAcquireLockAsync(string lockName, CancellationToken cancellationToken)
{
    var distributedLock = new SqlDistributedLock(lockName, _connectionString);

    var handler = await distributedLock.TryAcquireAsync(TimeSpan.FromSeconds(5), cancellationToken);

    if (handler != null)
    {
        return new LockHandlerToken(handler, handler.HandleLostToken);
    }

    return LockHandlerToken.None;
}

The LockHandlerToken only wrap the LockHandle provided by your lib as IDisposable (in this case SqlDistributedLockHandle).

Controller's action

using var lockHandlerToken = await _runManager.TryAcquireLockAsync(JobName, cancellationToken);

if (!lockHandlerToken.IsAcquired())
{
    // my logger
    return;
}

// do something

Using TaskScheduler.UnobservedTaskException callback I handling the exceptions.

I would like to know if this behavior is by design/expected by lib or not?

Exception

{
    "ClassName": "AggregateException",
    "FullClassName": "System.AggregateException",
    "InnerError": {
        "ClassName": "SqlException",
        "FullClassName": "Microsoft.Data.SqlClient.SqlException",
        "InnerError": null,
        "Message": "The request failed to run because the batch is aborted, this can be caused by abort signal sent from client, or another request is running in the same session, which makes the session busy.\r\nOperation cancelled by user.",
        "Source": "Core Microsoft SqlClient Data Provider",
        "StackTrace": "   at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)\r\n   at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)\r\n   at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)\r\n   at Microsoft.Data.SqlClient.SqlCommand.CompleteAsyncExecuteReader(Boolean isInternal, Boolean forDescribeParameterEncryption)\r\n   at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, Boolean isInternal, String endMethod)\r\n   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)\r\n   at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)\r\n   at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)\r\n--- End of stack trace from previous location ---\r\n   at Medallion.Threading.Internal.Data.DatabaseCommand.InternalExecuteAndPropagateCancellationAsync[TState,TResult](TState state, Func`3 executeAsync, CancellationToken cancellationToken, Boolean isConnectionMonitoringQuery) in /_/DistributedLock.Core/Internal/Data/DatabaseCommand.cs:line 167\r\n   at Medallion.Threading.Internal.Data.DatabaseCommand.ExecuteAsync[TResult](Func`3 executeAsync, Func`2 executeSync, CancellationToken cancellationToken, Boolean disallowAsyncCancellation, Boolean isConnectionMonitoringQuery) in /_/DistributedLock.Core/Internal/Data/DatabaseCommand.cs:line 92\r\n   at Medallion.Threading.SqlServer.SqlDatabaseConnection.SleepAsync(TimeSpan sleepTime, CancellationToken cancellationToken, Func`3 executor) in /_/src/DistributedLock.SqlServer/SqlDatabaseConnection.cs:line 65"
    },
    "Message": "A Task's exception(s) were not observed either by Waiting on the Task or accessing its Exception property. As a result, the unobserved exception was rethrown by the finalizer thread. (The request failed to run because the batch is aborted, this can be caused by abort signal sent from client, or another request is running in the same session, which makes the session busy.\r\nOperation cancelled by user.)",
    "Source": null,
    "StackTrace": null
}

More readable stacktrace:

at Microsoft.Data.SqlClient.TdsParser.ThrowExceptionAndWarning(TdsParserStateObject stateObj, Boolean callerHasConnectionLock, Boolean asyncClose)
at Microsoft.Data.SqlClient.TdsParser.TryRun(RunBehavior runBehavior, SqlCommand cmdHandler, SqlDataReader dataStream, BulkCopySimpleResultSet bulkCopyHandler, TdsParserStateObject stateObj, Boolean& dataReady)
at Microsoft.Data.SqlClient.SqlCommand.FinishExecuteReader(SqlDataReader ds, RunBehavior runBehavior, String resetOptionsString, Boolean isInternal, Boolean forDescribeParameterEncryption, Boolean shouldCacheForAlwaysEncrypted)
at Microsoft.Data.SqlClient.SqlCommand.CompleteAsyncExecuteReader(Boolean isInternal, Boolean forDescribeParameterEncryption)
at Microsoft.Data.SqlClient.SqlCommand.InternalEndExecuteNonQuery(IAsyncResult asyncResult, Boolean isInternal, String endMethod)
at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryInternal(IAsyncResult asyncResult)
at Microsoft.Data.SqlClient.SqlCommand.EndExecuteNonQueryAsync(IAsyncResult asyncResult)
at System.Threading.Tasks.TaskFactory`1.FromAsyncCoreLogic(IAsyncResult iar, Func`2 endFunction, Action`1 endAction, Task`1 promise, Boolean requiresSynchronization)
--- End of stack trace from previous location ---
at Medallion.Threading.Internal.Data.DatabaseCommand.InternalExecuteAndPropagateCancellationAsync[TState,TResult](TState state, Func`3 executeAsync, CancellationToken cancellationToken, Boolean isConnectionMonitoringQuery) in /_/DistributedLock.Core/Internal/Data/DatabaseCommand.cs:line 167
at Medallion.Threading.Internal.Data.DatabaseCommand.ExecuteAsync[TResult](Func`3 executeAsync, Func`2 executeSync, CancellationToken cancellationToken, Boolean disallowAsyncCancellation, Boolean isConnectionMonitoringQuery) in /_/DistributedLock.Core/Internal/Data/DatabaseCommand.cs:line 92
at Medallion.Threading.SqlServer.SqlDatabaseConnection.SleepAsync(TimeSpan sleepTime, CancellationToken cancellationToken, Func`3 executor) in /_/src/DistributedLock.SqlServer/SqlDatabaseConnection.cs:line 65
madelson commented 3 months ago

@nelsoncvjr-stone thanks for the detailed report!

return new LockHandlerToken(handler, handler.HandleLostToken);

By accessing HandleLostToken you're forcing the library to start proactively tracking the state of the underlying connection. However, it's not obvious from your code that you actually need this. I would suggest either removing the wrapper class or making the wrapper class access the token lazily so that you only pay this price when you are actually going to use the HandleLostToken in your caller.

I would like to know if this behavior is by design/expected by lib or not?

So far, I have not made "avoid UnobservedTaskException" a design constraint for this library. There are cases where the library spins off background tasks that could fail under normal conditions such as monitoring connection drops which might get interrupted by the application's desire to release the lock (and therefore close the connection).

In many cases, I've endeavored to minimize the overhead of thrown exceptions via an internal TryAwait() method. However, this issue revealed to me that TryAwait() was not properly observing the exception so it would still get reported as unobserved.

I've pushed a fix for that issue to the release-2.4 branch; it will be included in the next release of the library.

That said, I have not audited the entire library for other cases where UnobservedTaskException might fire spuriously; I'd welcome issues that identify more scenarios though.

nelsoncvjr-stone commented 3 months ago

Thanks for your attention @madelson!