HangfireIO / HangfireIO/Hangfire
SqlServerDistributedLock won't work correctly in async task
- Dominant language
- C#
- Stars
- 10.1k
- Forks
- 1.8k
- Avg merge
- 1h 19m
- Merged PRs (30d)
- 1
Description
It is currently not an issue for `DisableConcurrentExecutionAttribute`, since it wraps a parent task (which Hangfire runs synchronously), but it can become one in the future.
Consider the following test:
``` c#
private async Task AsyncChildTask()
{
using (var connection = ConnectionUtils.CreateConnection())
{
var storage = CreateStorage(connection);
using (var @lock = new SqlServerDistributedLock(storage, "test", _timeout))
{
var AcquiredLocksField = typeof(SqlServerDistributedLock)
.GetField("AcquiredLocks", BindingFlags.Static | BindingFlags.NonPublic);
var AcquiredLocks = (ThreadLocal>)AcquiredLocksField.GetValue(null);
Assert.True(AcquiredLocks.Value.ContainsKey("test"));
await Task.Yield();
Assert.True(AcquiredLocks.Value.ContainsKey("test"), "Lock is not owned"); // False!
}
});
}
[Fact]
public void ParentTaskCallingAsyncChild()
{
Task.WaitAll(AsyncChildTask());
}
```
It will fail the second assertion, because task may continue on different thread, so using `ThreadLocal<>` is incorrect for storing owned locks. You should use `AsyncLocal<>` instead (`CallContext` on older frameworks).
Contributor guide
Research direction
Start at the SqlServerDistributedLock implementation and inspect how its AcquiredLocks state is stored across asynchronous continuations. Reproduce the issue with the supplied AsyncChildTask and ParentTaskCallingAsyncChild test; done means the lock remains owned after Task.Yield and the test passes without losing ownership.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 48/100