Merge pull request #135 from madelson/release-2.3.1

madelson · web-flow · commit 7fb18b15e80a · 2022-07-09T09:49:23.000-04:00
Release 2.3.1
diff --git a/DistributedLock.Core/DistributedLock.Core.csproj b/DistributedLock.Core/DistributedLock.Core.csproj
@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.4</Version>
+    <Version>1.0.5</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Core interfaces and utilities that support the DistributedLock.* family of packages</Description>
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git 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
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git a/DistributedLock.MySql/DistributedLock.MySql.csproj b/DistributedLock.MySql/DistributedLock.MySql.csproj
@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.0</Version>
+    <Version>1.0.1</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Provides a distributed lock implementation based on MySql</Description>
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git a/DistributedLock.Oracle/DistributedLock.Oracle.csproj b/DistributedLock.Oracle/DistributedLock.Oracle.csproj
@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.0</Version>
+    <Version>1.0.1</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Provides a distributed lock implementation based on Oracle Database</Description>
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git a/DistributedLock.Postgres/DistributedLock.Postgres.csproj b/DistributedLock.Postgres/DistributedLock.Postgres.csproj
@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.2</Version>
+    <Version>1.0.3</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Provides a distributed lock implementation based on Postgresql</Description>
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git a/DistributedLock.Redis/DistributedLock.Redis.csproj b/DistributedLock.Redis/DistributedLock.Redis.csproj
@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.1</Version>
+    <Version>1.0.2</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Provides distributed locking primitives based on Redis</Description>
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git a/DistributedLock.Redis/RedisDistributedSynchronizationOptionsBuilder.cs b/DistributedLock.Redis/RedisDistributedSynchronizationOptionsBuilder.cs
@@ -145,12 +145,15 @@ internal static RedisDistributedLockOptions GetOptions(Action<RedisDistributedSy
             TimeoutValue extensionCadence;
             if (options?._extensionCadence is { } specifiedExtensionCadence)
             {
+                // Note: we do not allow for disabling auto-extension here because it leads to traps
+                // where people might abandon the handle and then have it be closed due to GC.
+                // See discussion here: https://github.com/madelson/DistributedLock/issues/130.
                 if (specifiedExtensionCadence.CompareTo(minValidityTime) >= 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
@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.1</Version>
+    <Version>1.0.2</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Provides a distributed lock implementation based on SQL Server</Description>
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git a/DistributedLock.Tests/Tests/Core/Data/DatabaseConnectionTest.cs 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
+    {
+        /// <summary>
+        /// Reproduces the root cause of https://github.com/madelson/DistributedLock/issues/133
+        /// </summary>
+        [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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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==");
         }
 
+        /// <summary>
+        /// 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().
+        /// </summary>
+        [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
@@ -10,7 +10,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.0</Version>
+    <Version>1.0.1</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Provides a distributed lock implementation based on global WaitHandle objects in Windows</Description>
@@ -32,6 +32,9 @@
     <TreatSpecificWarningsAsErrors />
     <!-- see https://github.com/dotnet/sdk/issues/2679 -->
     <DebugType>embedded</DebugType>
+    <!-- see https://mitchelsellers.com/blog/article/net-5-deterministic-builds-source-linking -->
+    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
+    <EmbedUntrackedSources>true</EmbedUntrackedSources>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
diff --git a/DistributedLock.WaitHandles/WaitHandleExtensions.cs b/DistributedLock.WaitHandles/WaitHandleExtensions.cs
diff --git a/DistributedLock.ZooKeeper/DistributedLock.ZooKeeper.csproj b/DistributedLock.ZooKeeper/DistributedLock.ZooKeeper.csproj
diff --git a/DistributedLock/DistributedLock.csproj b/DistributedLock/DistributedLock.csproj
diff --git a/README.md b/README.md

Original file line number	Diff line number	Diff line change
`@@ -145,12 +145,15 @@ internal static RedisDistributedLockOptions GetOptions(Action<RedisDistributedSy`
`145`	`145`	`TimeoutValue extensionCadence;`
`146`	`146`	`if (options?._extensionCadence is { } specifiedExtensionCadence)`
`147`	`147`	`{`
	`148`	`+ // Note: we do not allow for disabling auto-extension here because it leads to traps`
	`149`	`+ // where people might abandon the handle and then have it be closed due to GC.`
	`150`	`+ // See discussion here: https://github.com/madelson/DistributedLock/issues/130.`
`148`	`151`	`if (specifiedExtensionCadence.CompareTo(minValidityTime) >= 0)`
`149`	`152`	`{`
`150`	`153`	`throw new ArgumentOutOfRangeException(`
`151`	`154`	`nameof(extensionCadence),`
`152`	`155`	`specifiedExtensionCadence.TimeSpan,`
`153`		`- $"{nameof(extensionCadence)} must be less than {nameof(expiry)} ({expiry.TimeSpan}). To disable auto-extension, specify {nameof(Timeout)}.{nameof(Timeout.InfiniteTimeSpan)}"`
	`156`	`+ $"{nameof(extensionCadence)} must be less than {nameof(expiry)} ({expiry.TimeSpan})"`
`154`	`157`	`);`
`155`	`158`	`}`
`156`	`159`	`extensionCadence = specifiedExtensionCadence;`