From 22b9c6a18c0b524573236e6b208b448bd884bc81 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Wed, 6 Jul 2022 08:20:52 -0400 Subject: [PATCH 01/12] Ensure that ConnectionMonitor does not unsubscribe from state changes until disposal. Fix #133 --- .../Internal/Data/ConnectionMonitor.cs | 11 +++--- .../Tests/Core/Data/DatabaseConnectionTest.cs | 38 +++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) create mode 100644 DistributedLock.Tests/Tests/Core/Data/DatabaseConnectionTest.cs 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.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))); + } + } +} From f24c1b2ad6092003425b5f13f28bd0e6d9beec1d Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Wed, 6 Jul 2022 08:22:25 -0400 Subject: [PATCH 02/12] Additional CI markers --- .../Tests/MySql/MySqlConnectionOptionsBuilderTest.cs | 1 + .../Tests/Oracle/OracleConnectionOptionsBuilderTest.cs | 1 + .../Tests/Postgres/PostgresConnectionOptionsBuilderTest.cs | 1 + .../Tests/SqlServer/SqlConnectionOptionsBuilderTest.cs | 1 + 4 files changed, 4 insertions(+) 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] From bb5f5bebffc690510ab62d1e04b6d5e1004bc735 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Thu, 7 Jul 2022 08:17:21 -0400 Subject: [PATCH 03/12] Bump core version --- DistributedLock.Core/DistributedLock.Core.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DistributedLock.Core/DistributedLock.Core.csproj b/DistributedLock.Core/DistributedLock.Core.csproj index 37e245bb..ccc21e90 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 From ab10b00d7bcd1e55dfc23f2e4c88300a1eecaec9 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Thu, 7 Jul 2022 08:21:25 -0400 Subject: [PATCH 04/12] Remove confusing error message in Redis options that implied the ability to disable auto-extension. Fix #130 --- .../RedisDistributedSynchronizationOptionsBuilder.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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; From 4c42a795b612d891a12b375434187f24dcc28625 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 8 Jul 2022 14:31:45 -0400 Subject: [PATCH 05/12] Resignal waitHandle if we choose to ignore its signal due to cancellation. Fix #120 --- .../WaitHandleDistributedSemaphoreTest.cs | 36 +++++++++++++++++ .../WaitHandleExtensions.cs | 39 +++++++++++++++++-- 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs b/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs index f2c6665e..306f1178 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 @@ -78,6 +79,41 @@ public void TestGetSafeLockNameCompat() .ShouldEqual(@"Global\xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxsrCnXZ1XHiT//dOSBfAU0iC4Gtnlr0dQACBUK8Ev2OdRYJ9jcvbiqVCv/rjyPemTW9AvOonkdr0B2bG04gmeYA=="); } + /// + /// Reproduces https://github.com/madelson/DistributedLock/issues/120 + /// + [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)); + + for (var i = 0; i < 50; ++i) + { + using var barrier = new Barrier(2); + using var source = new CancellationTokenSource(); + var acquireTask = Task.Run(async () => + { + barrier.SignalAndWait(); + try + { + if (async) { await using var _ = await semaphore.AcquireAsync(cancellationToken: source.Token); } + else { using var _ = semaphore.Acquire(cancellationToken: source.Token); } + } + catch when (source.Token.IsCancellationRequested) { } + }); + var cancelTask = Task.Run(() => + { + barrier.SignalAndWait(); + source.Cancel(); + }); + await Task.WhenAll(acquireTask, 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( nameStyle == NameStyle.AddPrefix ? DistributedWaitHandleHelpers.GlobalPrefix + name : name, diff --git a/DistributedLock.WaitHandles/WaitHandleExtensions.cs b/DistributedLock.WaitHandles/WaitHandleExtensions.cs index 16ce40da..c513a257 100644 --- a/DistributedLock.WaitHandles/WaitHandleExtensions.cs +++ b/DistributedLock.WaitHandles/WaitHandleExtensions.cs @@ -42,6 +42,8 @@ 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(waitHandle is EventWaitHandle or Semaphore); // keep in sync with Resignal() + RegisteredWaitHandle? registeredHandle = null; CancellationTokenRegistration tokenRegistration = default; try @@ -50,13 +52,13 @@ private static async ValueTask InternalWaitOneAsync(this WaitHandle waitHa // 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 ); @@ -71,6 +73,37 @@ private static async ValueTask InternalWaitOneAsync(this WaitHandle waitHa 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). + } } } } From b86e691fd967aa666659725169c52e72fdcb5309 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 8 Jul 2022 18:41:04 -0400 Subject: [PATCH 06/12] Version bumps for 2.3.1 --- DistributedLock.MySql/DistributedLock.MySql.csproj | 2 +- DistributedLock.Oracle/DistributedLock.Oracle.csproj | 2 +- DistributedLock.Postgres/DistributedLock.Postgres.csproj | 2 +- DistributedLock.SqlServer/DistributedLock.SqlServer.csproj | 2 +- DistributedLock/DistributedLock.csproj | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/DistributedLock.MySql/DistributedLock.MySql.csproj b/DistributedLock.MySql/DistributedLock.MySql.csproj index bc66182f..df815249 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 diff --git a/DistributedLock.Oracle/DistributedLock.Oracle.csproj b/DistributedLock.Oracle/DistributedLock.Oracle.csproj index 3be6fe94..46fee21c 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 diff --git a/DistributedLock.Postgres/DistributedLock.Postgres.csproj b/DistributedLock.Postgres/DistributedLock.Postgres.csproj index 7d1aec27..a5d7eaba 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 diff --git a/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj b/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj index 5eac9c35..6d087aeb 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 diff --git a/DistributedLock/DistributedLock.csproj b/DistributedLock/DistributedLock.csproj index 4b62fa9a..84927b82 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. From 83ea7303847b746f163597b9a60bdc6d65e57850 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 8 Jul 2022 18:50:21 -0400 Subject: [PATCH 07/12] More version bumps --- DistributedLock.Redis/DistributedLock.Redis.csproj | 2 +- DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DistributedLock.Redis/DistributedLock.Redis.csproj b/DistributedLock.Redis/DistributedLock.Redis.csproj index 88bc955c..f8c244dc 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 diff --git a/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj b/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj index db7dbb67..c4cbc22b 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 From f662185f77b9623a10dda7269072d06f46bcca30 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 8 Jul 2022 18:50:27 -0400 Subject: [PATCH 08/12] Release notes --- README.md | 4 ++++ 1 file changed, 4 insertions(+) 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) From 9e6849ad5e32bf0ac008f872590ad7fd9ad42ed4 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 8 Jul 2022 18:58:35 -0400 Subject: [PATCH 09/12] Enable CI build flag for Core --- DistributedLock.Core/DistributedLock.Core.csproj | 1 + 1 file changed, 1 insertion(+) diff --git a/DistributedLock.Core/DistributedLock.Core.csproj b/DistributedLock.Core/DistributedLock.Core.csproj index ccc21e90..bee32015 100644 --- a/DistributedLock.Core/DistributedLock.Core.csproj +++ b/DistributedLock.Core/DistributedLock.Core.csproj @@ -32,6 +32,7 @@ embedded + true From 15cd9570b3a38a9fe4d167f0c07899b94ab63ac6 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 8 Jul 2022 19:04:31 -0400 Subject: [PATCH 10/12] Pass all validation checks in NuGet package explorer --- DistributedLock.Core/DistributedLock.Core.csproj | 2 ++ DistributedLock.FileSystem/DistributedLock.FileSystem.csproj | 3 +++ DistributedLock.MySql/DistributedLock.MySql.csproj | 3 +++ DistributedLock.Oracle/DistributedLock.Oracle.csproj | 3 +++ DistributedLock.Postgres/DistributedLock.Postgres.csproj | 3 +++ DistributedLock.Redis/DistributedLock.Redis.csproj | 3 +++ DistributedLock.SqlServer/DistributedLock.SqlServer.csproj | 3 +++ DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj | 3 +++ DistributedLock.ZooKeeper/DistributedLock.ZooKeeper.csproj | 3 +++ 9 files changed, 26 insertions(+) diff --git a/DistributedLock.Core/DistributedLock.Core.csproj b/DistributedLock.Core/DistributedLock.Core.csproj index bee32015..2bb6ba2d 100644 --- a/DistributedLock.Core/DistributedLock.Core.csproj +++ b/DistributedLock.Core/DistributedLock.Core.csproj @@ -32,7 +32,9 @@ embedded + true + true 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 df815249..0d0c5f28 100644 --- a/DistributedLock.MySql/DistributedLock.MySql.csproj +++ b/DistributedLock.MySql/DistributedLock.MySql.csproj @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Oracle/DistributedLock.Oracle.csproj b/DistributedLock.Oracle/DistributedLock.Oracle.csproj index 46fee21c..e7f7c71e 100644 --- a/DistributedLock.Oracle/DistributedLock.Oracle.csproj +++ b/DistributedLock.Oracle/DistributedLock.Oracle.csproj @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Postgres/DistributedLock.Postgres.csproj b/DistributedLock.Postgres/DistributedLock.Postgres.csproj index a5d7eaba..951424d4 100644 --- a/DistributedLock.Postgres/DistributedLock.Postgres.csproj +++ b/DistributedLock.Postgres/DistributedLock.Postgres.csproj @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.Redis/DistributedLock.Redis.csproj b/DistributedLock.Redis/DistributedLock.Redis.csproj index f8c244dc..ebdbd12f 100644 --- a/DistributedLock.Redis/DistributedLock.Redis.csproj +++ b/DistributedLock.Redis/DistributedLock.Redis.csproj @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj b/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj index 6d087aeb..abcc26fc 100644 --- a/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj +++ b/DistributedLock.SqlServer/DistributedLock.SqlServer.csproj @@ -32,6 +32,9 @@ embedded + + true + true diff --git a/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj b/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj index c4cbc22b..508bd1bd 100644 --- a/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj +++ b/DistributedLock.WaitHandles/DistributedLock.WaitHandles.csproj @@ -32,6 +32,9 @@ embedded + + true + true 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 From 862e136554d799ff967723ce2d11ab88ab9b8d8c Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Fri, 8 Jul 2022 19:19:38 -0400 Subject: [PATCH 11/12] CI build for umbrella package --- DistributedLock/DistributedLock.csproj | 3 +++ 1 file changed, 3 insertions(+) diff --git a/DistributedLock/DistributedLock.csproj b/DistributedLock/DistributedLock.csproj index 84927b82..536c39fd 100644 --- a/DistributedLock/DistributedLock.csproj +++ b/DistributedLock/DistributedLock.csproj @@ -33,6 +33,9 @@ embedded + + true + true From f4a3401d8fa1236978d4b9678b5012262fa876a1 Mon Sep 17 00:00:00 2001 From: Michael Adelson Date: Sat, 9 Jul 2022 08:53:46 -0400 Subject: [PATCH 12/12] Further race condition handling for #120 --- .../WaitHandleDistributedSemaphoreTest.cs | 49 ++++++++++++++----- .../WaitHandleExtensions.cs | 49 +++++++++++++++---- 2 files changed, 77 insertions(+), 21 deletions(-) diff --git a/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs b/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs index 306f1178..16f0dbab 100644 --- a/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs +++ b/DistributedLock.Tests/Tests/WaitHandles/WaitHandleDistributedSemaphoreTest.cs @@ -48,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(); @@ -80,7 +80,10 @@ public void TestGetSafeLockNameCompat() } /// - /// Reproduces https://github.com/madelson/DistributedLock/issues/120 + /// 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) @@ -88,26 +91,50 @@ 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 barrier = new Barrier(2); - using var source = new CancellationTokenSource(); + 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 () => { - barrier.SignalAndWait(); try - { - if (async) { await using var _ = await semaphore.AcquireAsync(cancellationToken: source.Token); } - else { using var _ = semaphore.Acquire(cancellationToken: source.Token); } + { + 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 when (source.Token.IsCancellationRequested) { } + 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, cancelTask); + await Task.WhenAll(acquireTask, releaseTask, cancelTask); } await using var handle = await semaphore.TryAcquireAsync(); @@ -115,7 +142,7 @@ public async Task TestCancellationDoesNotLeadToLostSignal([Values] bool async) } 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/WaitHandleExtensions.cs b/DistributedLock.WaitHandles/WaitHandleExtensions.cs index c513a257..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,13 +57,14 @@ 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(waitHandle is EventWaitHandle or Semaphore); // keep in sync with Resignal() + 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( @@ -66,11 +82,24 @@ 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(); }