diff --git a/DistributedLock.Core/DistributedLock.Core.csproj b/DistributedLock.Core/DistributedLock.Core.csproj index 37e245bb..2bb6ba2d 100644 --- a/DistributedLock.Core/DistributedLock.Core.csproj +++ b/DistributedLock.Core/DistributedLock.Core.csproj @@ -10,7 +10,7 @@ - 1.0.4 + 1.0.5 1.0.0.0 Michael Adelson Core interfaces and utilities that support the DistributedLock.* family of packages @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Core/Internal/Data/ConnectionMonitor.cs b/DistributedLock.Core/Internal/Data/ConnectionMonitor.cs index ece63bd2..4830fc24 100644 --- a/DistributedLock.Core/Internal/Data/ConnectionMonitor.cs +++ b/DistributedLock.Core/Internal/Data/ConnectionMonitor.cs @@ -253,9 +253,10 @@ private async ValueTask StopOrDisposeAsync(bool isDispose) // the state to disposed above which the monitoring loop will check if it // takes over the Cancel() thread. this._monitorStateChangedTokenSource?.Cancel(); - - // unsubscribe from state change tracking - if (this._stateChangedHandler != null + + // If disposing, unsubscribe from state change tracking. + if (isDispose + && this._stateChangedHandler != null && this._weakConnection.TryGetTarget(out var connection)) { ((DbConnection)connection.InnerConnection).StateChange -= this._stateChangedHandler; @@ -424,7 +425,7 @@ public MonitoringHandle(ConnectionMonitor keepaliveHelper, CancellationToken can private sealed class AlreadyCanceledHandle : IDatabaseConnectionMonitoringHandle { - private readonly CancellationTokenSource _cancellationTokenSource = new CancellationTokenSource(); + private readonly CancellationTokenSource _cancellationTokenSource = new(); public AlreadyCanceledHandle() { @@ -438,7 +439,7 @@ public AlreadyCanceledHandle() private sealed class NullHandle : IDatabaseConnectionMonitoringHandle { - public static readonly NullHandle Instance = new NullHandle(); + public static readonly NullHandle Instance = new(); private NullHandle() { } diff --git a/DistributedLock.FileSystem/DistributedLock.FileSystem.csproj b/DistributedLock.FileSystem/DistributedLock.FileSystem.csproj index 6c342f04..7e72e548 100644 --- a/DistributedLock.FileSystem/DistributedLock.FileSystem.csproj +++ b/DistributedLock.FileSystem/DistributedLock.FileSystem.csproj @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.MySql/DistributedLock.MySql.csproj b/DistributedLock.MySql/DistributedLock.MySql.csproj index bc66182f..0d0c5f28 100644 --- a/DistributedLock.MySql/DistributedLock.MySql.csproj +++ b/DistributedLock.MySql/DistributedLock.MySql.csproj @@ -10,7 +10,7 @@ - 1.0.0 + 1.0.1 1.0.0.0 Michael Adelson Provides a distributed lock implementation based on MySql @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Oracle/DistributedLock.Oracle.csproj b/DistributedLock.Oracle/DistributedLock.Oracle.csproj index 3be6fe94..e7f7c71e 100644 --- a/DistributedLock.Oracle/DistributedLock.Oracle.csproj +++ b/DistributedLock.Oracle/DistributedLock.Oracle.csproj @@ -10,7 +10,7 @@ - 1.0.0 + 1.0.1 1.0.0.0 Michael Adelson Provides a distributed lock implementation based on Oracle Database @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Postgres/DistributedLock.Postgres.csproj b/DistributedLock.Postgres/DistributedLock.Postgres.csproj index 7d1aec27..951424d4 100644 --- a/DistributedLock.Postgres/DistributedLock.Postgres.csproj +++ b/DistributedLock.Postgres/DistributedLock.Postgres.csproj @@ -10,7 +10,7 @@ - 1.0.2 + 1.0.3 1.0.0.0 Michael Adelson Provides a distributed lock implementation based on Postgresql @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Redis/DistributedLock.Redis.csproj b/DistributedLock.Redis/DistributedLock.Redis.csproj index 88bc955c..ebdbd12f 100644 --- a/DistributedLock.Redis/DistributedLock.Redis.csproj +++ b/DistributedLock.Redis/DistributedLock.Redis.csproj @@ -10,7 +10,7 @@ - 1.0.1 + 1.0.2 1.0.0.0 Michael Adelson Provides distributed locking primitives based on Redis @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Redis/RedisDistributedSynchronizationOptionsBuilder.cs b/DistributedLock.Redis/RedisDistributedSynchronizationOptionsBuilder.cs index c46bdf8c..f15c0803 100644 --- a/DistributedLock.Redis/RedisDistributedSynchronizationOptionsBuilder.cs +++ b/DistributedLock.Redis/RedisDistributedSynchronizationOptionsBuilder.cs @@ -145,12 +145,15 @@ internal static RedisDistributedLockOptions GetOptions(Action= 0) { throw new ArgumentOutOfRangeException( nameof(extensionCadence), specifiedExtensionCadence.TimeSpan, - $"{nameof(extensionCadence)} must be less than {nameof(expiry)} ({expiry.TimeSpan}). To disable auto-extension, specify {nameof(Timeout)}.{nameof(Timeout.InfiniteTimeSpan)}" + $"{nameof(extensionCadence)} must be less than {nameof(expiry)} ({expiry.TimeSpan})" ); } extensionCadence = specifiedExtensionCadence; diff --git a/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj b/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj index 5eac9c35..abcc26fc 100644 --- a/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj +++ b/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj @@ -10,7 +10,7 @@ - 1.0.1 + 1.0.2 1.0.0.0 Michael Adelson Provides a distributed lock implementation based on SQL Server @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Tests/Tests/Core/Data/DatabaseConnectionTest.cs b/DistributedLock.Tests/Tests/Core/Data/DatabaseConnectionTest.cs new file mode 100644 index 00000000..823dc27a --- /dev/null +++ b/DistributedLock.Tests/Tests/Core/Data/DatabaseConnectionTest.cs @@ -0,0 +1,38 @@ +using Medallion.Threading.Internal.Data; +using Medallion.Threading.SqlServer; +using Medallion.Threading.Tests.SqlServer; +using NUnit.Framework; +using System; +using System.Collections.Generic; +using System.Data.SqlClient; +using System.Linq; +using System.Text; +using System.Threading; +using System.Threading.Tasks; + +namespace Medallion.Threading.Tests.Core.Data +{ + public class DatabaseConnectionTest + { + /// + /// Reproduces the root cause of https://github.com/madelson/DistributedLock/issues/133 + /// + [Test] + public async Task TestConnectionMonitorStaysSubscribedAfterClose() + { + var db = new TestingSqlServerDb { ApplicationName = nameof(TestConnectionMonitorStaysSubscribedAfterClose) }; + + await using var connection = new SqlDatabaseConnection(db.ConnectionString); + + await connection.OpenAsync(CancellationToken.None); + connection.ConnectionMonitor.GetMonitoringHandle().Dispose(); // initialize monitoring + await connection.CloseAsync(); + + await connection.OpenAsync(CancellationToken.None); + using var handle = connection.ConnectionMonitor.GetMonitoringHandle(); + Assert.IsFalse(handle.ConnectionLostToken.IsCancellationRequested); + await db.KillSessionsAsync(db.ApplicationName, idleSince: null); + Assert.IsTrue(await TestHelper.WaitForAsync(() => new(handle.ConnectionLostToken.IsCancellationRequested), timeout: TimeSpan.FromSeconds(5))); + } + } +} diff --git a/DistributedLock.Tests/Tests/MySql/MySqlConnectionOptionsBuilderTest.cs b/DistributedLock.Tests/Tests/MySql/MySqlConnectionOptionsBuilderTest.cs index c543bb62..ea428d44 100644 --- a/DistributedLock.Tests/Tests/MySql/MySqlConnectionOptionsBuilderTest.cs +++ b/DistributedLock.Tests/Tests/MySql/MySqlConnectionOptionsBuilderTest.cs @@ -6,6 +6,7 @@ namespace Medallion.Threading.Tests.MySql { + [Category("CI")] public class MySqlConnectionOptionsBuilderTest { [Test] diff --git a/DistributedLock.Tests/Tests/Oracle/OracleConnectionOptionsBuilderTest.cs b/DistributedLock.Tests/Tests/Oracle/OracleConnectionOptionsBuilderTest.cs index 05e8d27d..806e79a9 100644 --- a/DistributedLock.Tests/Tests/Oracle/OracleConnectionOptionsBuilderTest.cs +++ b/DistributedLock.Tests/Tests/Oracle/OracleConnectionOptionsBuilderTest.cs @@ -7,6 +7,7 @@ namespace Medallion.Threading.Tests.Oracle { + [Category("CI")] public class OracleConnectionOptionsBuilderTest { [Test] diff --git a/DistributedLock.Tests/Tests/Postgres/PostgresConnectionOptionsBuilderTest.cs b/DistributedLock.Tests/Tests/Postgres/PostgresConnectionOptionsBuilderTest.cs index 4c051aa2..1bd14602 100644 --- a/DistributedLock.Tests/Tests/Postgres/PostgresConnectionOptionsBuilderTest.cs +++ b/DistributedLock.Tests/Tests/Postgres/PostgresConnectionOptionsBuilderTest.cs @@ -6,6 +6,7 @@ namespace Medallion.Threading.Tests.Postgres { + [Category("CI")] public class PostgresConnectionOptionsBuilderTest { [Test] diff --git a/DistributedLock.Tests/Tests/SqlServer/SqlConnectionOptionsBuilderTest.cs b/DistributedLock.Tests/Tests/SqlServer/SqlConnectionOptionsBuilderTest.cs index d8ac6c79..a5a465f1 100644 --- a/DistributedLock.Tests/Tests/SqlServer/SqlConnectionOptionsBuilderTest.cs +++ b/DistributedLock.Tests/Tests/SqlServer/SqlConnectionOptionsBuilderTest.cs @@ -6,6 +6,7 @@ namespace Medallion.Threading.Tests.SqlServer { + [Category("CI")] public class SqlConnectionOptionsBuilderTest { [Test] diff --git a/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs b/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs index f2c6665e..16f0dbab 100644 --- a/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs +++ b/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Text; +using System.Threading; using System.Threading.Tasks; namespace Medallion.Threading.Tests.WaitHandles @@ -47,7 +48,7 @@ public void TestMaxLengthNames() public async Task TestGarbageCollection() { var @lock = CreateAsLock("gc_test", NameStyle.AddPrefix); - WeakReference AbandonLock() => new WeakReference(@lock.Acquire()); + WeakReference AbandonLock() => new(@lock.Acquire()); var weakHandle = AbandonLock(); GC.Collect(); @@ -78,8 +79,70 @@ public void TestGetSafeLockNameCompat() .ShouldEqual(@"Global\xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxsrCnXZ1XHiT//dOSBfAU0iC4Gtnlr0dQACBUK8Ev2OdRYJ9jcvbiqVCv/rjyPemTW9AvOonkdr0B2bG04gmeYA=="); } + /// + /// Attempts to reproduce https://github.com/madelson/DistributedLock/issues/120. + /// + /// NOTE: in practice this race condition is so slim that to reproduce with any reliability requires + /// adding a call to Thread.Sleep(1) at the start of WaitHandleExtensions.Resignal(). + /// + [Test] + public async Task TestCancellationDoesNotLeadToLostSignal([Values] bool async) + { + var semaphore = new WaitHandleDistributedSemaphore(nameof(this.TestCancellationDoesNotLeadToLostSignal), 2); + await using var _ = await semaphore.AcquireAsync(TimeSpan.FromSeconds(1)); + + Random random = new(); + for (var i = 0; i < 50; ++i) + { + using var blockingHandle = semaphore.TryAcquire(TimeSpan.Zero); // claim the last slot on the semaphore + Assert.IsNotNull(blockingHandle); + + using CancellationTokenSource source = new(); + + using SemaphoreSlim acquiringEvent = new(initialCount: 0, maxCount: 1); + var acquireTask = Task.Run(async () => + { + try + { + if (async) + { + var acquireHandleTask = semaphore.AcquireAsync(TimeSpan.FromSeconds(30), source.Token); + acquiringEvent.Release(); + (await acquireHandleTask).Dispose(); + } + else + { + acquiringEvent.Release(); + semaphore.Acquire(TimeSpan.FromSeconds(30), source.Token).Dispose(); + } + } + catch (OperationCanceledException) { } + }); + await acquiringEvent.WaitAsync(); + Assert.IsFalse(acquireTask.IsCompleted); + + using Barrier barrier = new(participantCount: 2); + var releaseTask = Task.Run(() => + { + barrier.SignalAndWait(); + blockingHandle!.Dispose(); + }); + var cancelTask = Task.Run(() => + { + barrier.SignalAndWait(); + var yieldCount = random.Next(5, 25); + for (var i = 0; i < yieldCount; ++i) { Thread.Yield(); } + source.Cancel(); + }); + await Task.WhenAll(acquireTask, releaseTask, cancelTask); + } + + await using var handle = await semaphore.TryAcquireAsync(); + Assert.IsNotNull(handle); // if we lost even a single signal due to cancellation in the loop above, this will fail + } + private static WaitHandleDistributedSemaphore CreateAsLock(string name, NameStyle nameStyle) => - new WaitHandleDistributedSemaphore( + new( nameStyle == NameStyle.AddPrefix ? DistributedWaitHandleHelpers.GlobalPrefix + name : name, maxCount: 1, abandonmentCheckCadence: TimeSpan.FromSeconds(.3), diff --git a/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj b/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj index db7dbb67..508bd1bd 100644 --- a/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj +++ b/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj @@ -10,7 +10,7 @@ - 1.0.0 + 1.0.1 1.0.0.0 Michael Adelson Provides a distributed lock implementation based on global WaitHandle objects in Windows @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.WaitHandles/WaitHandleExtensions.cs b/DistributedLock.WaitHandles/WaitHandleExtensions.cs index 16ce40da..1fe7a7f6 100644 --- a/DistributedLock.WaitHandles/WaitHandleExtensions.cs +++ b/DistributedLock.WaitHandles/WaitHandleExtensions.cs @@ -9,9 +9,14 @@ internal static class WaitHandleExtensions { public static async ValueTask WaitOneAsync(this WaitHandle waitHandle, TimeoutValue timeout, CancellationToken cancellationToken) { - return SyncViaAsync.IsSynchronous - ? waitHandle.InternalWaitOne(timeout, cancellationToken) - : await waitHandle.InternalWaitOneAsync(timeout, cancellationToken).ConfigureAwait(false); + if (timeout.IsZero || SyncViaAsync.IsSynchronous) + { + return waitHandle.InternalWaitOne(timeout, cancellationToken); + } + + // when doing an async wait, still do a quick sync check first with timeout zero to optimize the already-signaled case + return waitHandle.InternalWaitOne(TimeSpan.Zero, cancellationToken) + || await waitHandle.InternalWaitOneAsync(timeout, cancellationToken).ConfigureAwait(false); } private static bool InternalWaitOne(this WaitHandle waitHandle, TimeoutValue timeout, CancellationToken cancellationToken) @@ -25,6 +30,16 @@ private static bool InternalWaitOne(this WaitHandle waitHandle, TimeoutValue tim // ensures that we cancel cancellationToken.ThrowIfCancellationRequested(); + // optimize the already-signaled case + if (waitHandle.WaitOne(TimeSpan.Zero)) + { + return true; + } + if (timeout.IsZero) + { + return false; + } + // cancellable wait based on // http://www.thomaslevesque.com/2015/06/04/async-and-cancellation-support-for-wait-handles/ var index = WaitHandle.WaitAny(new[] { waitHandle, cancellationToken.WaitHandle }, timeout.InMilliseconds); @@ -42,21 +57,24 @@ private static bool InternalWaitOne(this WaitHandle waitHandle, TimeoutValue tim // based on http://www.thomaslevesque.com/2015/06/04/async-and-cancellation-support-for-wait-handles/ private static async ValueTask InternalWaitOneAsync(this WaitHandle waitHandle, TimeoutValue timeout, CancellationToken cancellationToken) { + Invariant.Require(!cancellationToken.CanBeCanceled || waitHandle is EventWaitHandle or Semaphore); // keep in sync with Resignal() + + var taskCompletionSource = new TaskCompletionSource(); + RegisteredWaitHandle? registeredHandle = null; CancellationTokenRegistration tokenRegistration = default; try { - var taskCompletionSource = new TaskCompletionSource(); // if, upon entering the method we are already both canceled and signaled, // putting this first ensures that we cancel tokenRegistration = cancellationToken.Register( - state => ((TaskCompletionSource)state).TrySetCanceled(), + static state => ((TaskCompletionSource)state).TrySetCanceled(), state: taskCompletionSource ); registeredHandle = ThreadPool.RegisterWaitForSingleObject( waitHandle, - (state, timedOut) => ((TaskCompletionSource)state).TrySetResult(!timedOut), - state: taskCompletionSource, + static (state, timedOut) => OnSignaled(state, timedOut), + state: Tuple.Create(taskCompletionSource, waitHandle), millisecondsTimeOutInterval: timeout.InMilliseconds, executeOnlyOnce: true ); @@ -64,13 +82,57 @@ private static async ValueTask InternalWaitOneAsync(this WaitHandle waitHa } finally { - // this is different from the referenced site, but I think this is more correct: - // the handle passed to unregister is a handle to be signaled, not the one to unregister - // (that one is already captured by the registered handle). See - // http://referencesource.microsoft.com/#mscorlib/system/threading/threadpool.cs,065408fc096354fd - registeredHandle?.Unregister(null); + if (registeredHandle != null) + { + if (taskCompletionSource.Task.IsCanceled) + { + // If the task got canceled, then there is a slim chance of a race condition where + // the wait callback is still running, and hasn't re-signaled the handle yet. If we + // return before that point then we might dispose the handle, before getting to re-signal + // it. To prevent that, we pass in an MRE which will be signaled when the reservation fully + // completes and we wait for that signal before returning. + using ManualResetEvent unregisterCompleteEvent = new(initialState: false); + registeredHandle.Unregister(unregisterCompleteEvent); + await unregisterCompleteEvent.WaitOneAsync(Timeout.InfiniteTimeSpan, CancellationToken.None).ConfigureAwait(false); + } + else + { + registeredHandle.Unregister(null); + } + } tokenRegistration.Dispose(); } + + static void OnSignaled(object state, bool timedOut) + { + var (taskCompletionSource, waitHandle) = (Tuple, WaitHandle>)state; + if (!taskCompletionSource.TrySetResult(!timedOut) && !timedOut && taskCompletionSource.Task.IsCanceled) + { + // If we received a signal (not a timeout) and we lost the race with cancellation, resignal + // the handle to avoid the signal being lost. See https://github.com/madelson/DistributedLock/issues/120 + Resignal(waitHandle); + } + } + } + + private static void Resignal(WaitHandle waitHandle) + { + try + { + if (waitHandle is EventWaitHandle @event) + { + @event.Set(); + } + else if (waitHandle is Semaphore semaphore) + { + semaphore.Release(); + } + } + catch + { + // Since this method runs in a threadpool thread, we don't want it to throw + // even if the methods above fail (e.g. with SemaphoreFullException). + } } } } diff --git a/DistributedLock.ZooKeeper/DistributedLock.ZooKeeper.csproj b/DistributedLock.ZooKeeper/DistributedLock.ZooKeeper.csproj index db14c847..67cbf653 100644 --- a/DistributedLock.ZooKeeper/DistributedLock.ZooKeeper.csproj +++ b/DistributedLock.ZooKeeper/DistributedLock.ZooKeeper.csproj @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock/DistributedLock.csproj b/DistributedLock/DistributedLock.csproj index 4b62fa9a..536c39fd 100644 --- a/DistributedLock/DistributedLock.csproj +++ b/DistributedLock/DistributedLock.csproj @@ -10,7 +10,7 @@ - 2.3.0 + 2.3.1 2.0.0.0 Michael Adelson Provides easy-to-use mutexes, reader-writer locks, and semaphores that can synchronize across processes and machines. This is an umbrella package that brings in the entire family of DistributedLock.* packages (e. g. DistributedLock.SqlServer) as references. Those packages can also be installed individually. @@ -33,6 +33,9 @@ embedded + + true + true diff --git a/README.md b/README.md index 75865b6a..8c83ac00 100644 --- a/README.md +++ b/README.md @@ -137,6 +137,10 @@ public class SomeService Contributions are welcome! If you are interested in contributing towards a new or existing issue, please let me know via comments on the issue so that I can help you get started and avoid wasted effort on your part. ## Release notes +- 2.3.1 + - Fixed concurrency issue with `HandleLostToken` for relational database locks ([#133](https://github.com/madelson/DistributedLock/issues/133), DistributedLock.Core 1.0.5, DistributedLock.MySql 1.0.1, DistributedLock.Oracle 1.0.1, DistributedLock.Postgres 1.0.3, DistributedLock.SqlServer 1.0.2). Thanks [@OskarKlintrot](https://github.com/OskarKlintrot) for testing! + - Fixed misleading error message why trying to disable auto-extension in Redis ([#130](https://github.com/madelson/DistributedLock/issues/130), DistributedLock.Redis 1.0.2) + - Fixed concurrency issue with canceling async waits on `WaitHandle`s ([#120](https://github.com/madelson/DistributedLock/issues/120), DistributedLock.WaitHandles 1.0.1) - 2.3.0 - Added Oracle-based implementation ([#45](https://github.com/madelson/DistributedLock/issues/45), DistributedLock.Oracle 1.0.0). Thanks [@odin568](https://github.com/odin568) for testing! - Made file-based locking more robust to transient `UnauthorizedAccessException`s ([#106](https://github.com/madelson/DistributedLock/issues/106) & [#109](https://github.com/madelson/DistributedLock/issues/109), DistributedLock.FileSystem 1.0.1)