Merge pull request #232 from madelson/release-2.5.1

Release 2.5.1

Author: madelson

README.md

@@ -141,6 +141,13 @@ Contributions are welcome! If you are interested in contributing towards a new o
 Setup steps for working with the repository locally are documented [here](docs/Developing%20DistributedLock.md).
 
 ## Release notes
+- 2.5.1
+	- Increase efficiency of Azure blob locks when the blob does not exist. Thanks [@richardkooiman](https://github.com/richardkooiman) for implementing! ([#227](https://github.com/madelson/DistributedLock/pull/227), DistributedLock.Azure 1.0.2)
+	- Improve error handling in race condition scenarios for Azure blobs. Thanks [@MartinDembergerR9](https://github.com/MartinDembergerR9) for implementing! ([#228](https://github.com/madelson/DistributedLock/pull/228), DistributedLock.Azure 1.0.2)
+	- Bump Microsoft.Data.SqlClient to 5.2.2 to avoid vulnerability. Thanks [@steve85](https://github.com/steve85) for implementing! ([#229](https://github.com/madelson/DistributedLock/pull/229), DistributedLock.SqlServer 1.0.6)
+	- Bump Oracle.ManagedDataAccess to latest to avoid bringing in vulnerable packages (DistributedLock.Core 1.0.8, DistributedLock.Oracle 1.0.4)
+	- Bump Npgsql to latest patch to avoid bringing in vulnerable packages (DistributedLock.Postgres 1.2.1)
+	- Improve directory creation concurrency handling for `FileDistributedLock` (DistributedLock.FileSystem 1.0.3) 
 - 2.5
 	- Add support for creating Postgres locks off `DbDataSource` which is helpful for apps using `NpgsqlMultiHostDataSource`. Thanks [davidngjy](https://github.com/davidngjy) for implementing! ([#153](https://github.com/madelson/DistributedLock/issues/153), DistributedLock.Postgres 1.2.0)
 	- Upgrade Npgsql to 8.0.3 to avoid vulnerability. Thanks [@Meir017](https://github.com/Meir017)/[@davidngjy](https://github.com/davidngjy) for implementing! ([#218](https://github.com/madelson/DistributedLock/issues/218), DistributedLock.Postgres 1.2.0)
@@ -149,10 +156,10 @@ Setup steps for working with the repository locally are documented [here](docs/D
 - 2.4
 	- Add support for transaction-scoped locking in Postgres using `pg_advisory_xact_lock` which is helpful when using PgBouncer ([#168](https://github.com/madelson/DistributedLock/issues/168), DistributedLock.Postgres 1.1.0)
 	- Improve support for newer versions of StackExchange.Redis, especially when using the default backlog policy ([#162](https://github.com/madelson/DistributedLock/issues/162), DistributedLock.Redis 1.0.3). Thanks [@Bartleby2718](https://github.com/Bartleby2718) for helping with this!
-   - Drop `net461` support (`net462` remains supported). Thanks [@Bartleby2718](https://github.com/Bartleby2718) for implementing! 
+	- Drop `net461` support (`net462` remains supported). Thanks [@Bartleby2718](https://github.com/Bartleby2718) for implementing! 
 	- Reduce occurrence of `UnobservedTaskException`s thrown by the library ([#192](https://github.com/madelson/DistributedLock/issues/192), DistributedLock.Core 1.0.6)
 	- Update dependencies to modern versions without known issues/vulnerabilities ([#111](https://github.com/madelson/DistributedLock/issues/111)/[#177](https://github.com/madelson/DistributedLock/issues/177)/[#184](https://github.com/madelson/DistributedLock/issues/184)/[#185](https://github.com/madelson/DistributedLock/issues/185), all packages). Thanks [@Bartleby2718](https://github.com/Bartleby2718) for helping with this!
-	- Improve directory creation concurrency handling for `FileDistributedLock` on Linux/.NET 8 ([#195](), DistributedLock.FileSystem 1.0.2)
+	- Improve directory creation concurrency handling for `FileDistributedLock` on Linux/.NET 8 ([#195](https://github.com/madelson/DistributedLock/issues/195), DistributedLock.FileSystem 1.0.2)
 	- Allow using transaction-scoped locks in SQL Server without explicitly disabling multiplexing ([#189](https://github.com/madelson/DistributedLock/issues/189), DistributedLock.SqlServer 1.0.4)
 	- New API documentation on [dndocs](https://dndocs.com/). Thanks [@NeuroXiq](https://github.com/NeuroXiq)!
 	- New documentation for contributors to get the project running locally (see [Contributing](#contributing))

src/Directory.Packages.props

@@ -10,11 +10,11 @@
     <PackageVersion Include="Nullable" Version="1.3.1" Condition="'$(TargetFramework)' != 'netstandard2.1'" />
     <PackageVersion Include="MySqlConnector" Version="2.3.5" />
     <PackageVersion Include="NUnit.Analyzers" Version="4.1.0" />
-    <PackageVersion Include="Oracle.ManagedDataAccess.Core" Version="3.21.130" Condition="'$(TargetFramework)' == 'netstandard2.1'" />
-    <PackageVersion Include="Oracle.ManagedDataAccess" Version="21.13.0" Condition="'$(TargetFramework)' == 'net462'" />
-    <PackageVersion Include="Npgsql" Version="8.0.3" />
+    <PackageVersion Include="Oracle.ManagedDataAccess.Core" Version="23.6.1" Condition="'$(TargetFramework)' == 'netstandard2.1'" />
+    <PackageVersion Include="Oracle.ManagedDataAccess" Version="23.6.1" Condition="'$(TargetFramework)' == 'net472'" />
+    <PackageVersion Include="Npgsql" Version="8.0.6" />
     <PackageVersion Include="StackExchange.Redis" Version="2.7.27" />
-    <PackageVersion Include="Microsoft.Data.SqlClient" Version="5.2.1" />
+    <PackageVersion Include="Microsoft.Data.SqlClient" Version="5.2.2" />
     <PackageVersion Include="nunit" Version="3.14.0" />
     <PackageVersion Include="nunit3testadapter" Version="4.5.0" />
     <PackageVersion Include="Microsoft.NET.Test.SDK" Version="17.9.0" />
@@ -26,5 +26,6 @@
     <PackageVersion Include="IsExternalInit" Version="1.0.3" />
     <PackageVersion Include="Microsoft.Bcl.AsyncInterfaces" Version="8.0.0" Condition="'$(TargetFramework)' == 'netstandard2.0' OR '$(TargetFramework)' == 'net462'" />
     <PackageVersion Include="System.ValueTuple" Version="4.5.0" Condition="'$(TargetFramework)' == 'net462'" />
+    <PackageVersion Include="System.Private.Uri" Version="4.3.2" />
   </ItemGroup>
-</Project>
+</Project>

src/DistributedLock.Azure/AzureBlobLeaseDistributedLock.cs

@@ -12,7 +12,7 @@ namespace Medallion.Threading.Azure;
 public sealed partial class AzureBlobLeaseDistributedLock : IInternalDistributedLock<AzureBlobLeaseDistributedLockHandle>
 {
     /// <summary>
-    /// Metadata marker used to indicate that a blob was created for distributed locking and therefore 
+    /// Metadata marker used to indicate that a blob was created for distributed locking and therefore
     /// should be destroyed upon release
     /// </summary>
     private static readonly string CreatedMetadataKey = $"__DistributedLock";
@@ -51,9 +51,9 @@ internal static string GetSafeName(string name, BlobContainerClient blobContaine
     {
         var maxLength = IsStorageEmulator() ? 256 : 1024;
 
-        return DistributedLockHelpers.ToSafeName(name, maxLength, s => ConvertToValidName(s));
+        return DistributedLockHelpers.ToSafeName(name, maxLength, ConvertToValidName);
 
-        // check based on 
+        // check based on
         // https://docs.microsoft.com/en-us/azure/storage/common/storage-use-emulator#connect-to-the-emulator-account-using-the-well-known-account-name-and-key
         bool IsStorageEmulator() => blobContainerClient.Uri.IsAbsoluteUri
             && blobContainerClient.Uri.AbsoluteUri.StartsWith("http://127.0.0.1:10000/devstoreaccount1", StringComparison.Ordinal);
@@ -105,13 +105,15 @@ static string ConvertToValidName(string name)
         );
 
     private async ValueTask<AzureBlobLeaseDistributedLockHandle?> TryAcquireAsync(
-        BlobLeaseClientWrapper leaseClient, 
+        BlobLeaseClientWrapper leaseClient,
         CancellationToken cancellationToken,
         bool isRetryAfterCreate)
     {
-        try  { await leaseClient.AcquireAsync(this._options.duration, cancellationToken).ConfigureAwait(false); }
-        catch (RequestFailedException acquireException)
+        using var response = await leaseClient.AcquireAsync(this._options.duration, cancellationToken).ConfigureAwait(false);
+        if (response.IsError)
         {
+            var acquireException = new RequestFailedException(response);
+
             if (acquireException.ErrorCode == AzureErrors.LeaseAlreadyPresent) { return null; }
 
             if (acquireException.ErrorCode == AzureErrors.BlobNotFound)
@@ -126,7 +128,7 @@ static string ConvertToValidName(string name)
                 {
                     // handle the race condition where we try to create and someone else creates it first
                     return createException.ErrorCode == AzureErrors.LeaseIdMissing
-                        ? default(AzureBlobLeaseDistributedLockHandle?)
+                        ? default
                         : throw new AggregateException($"Blob {this._blobClient.Name} does not exist and could not be created. See inner exceptions for details", acquireException, createException);
                 }
 
@@ -135,6 +137,11 @@ static string ConvertToValidName(string name)
                 {
                     // if the retry fails and we created, attempt deletion to clean things up
                     try { await this._blobClient.DeleteIfExistsAsync().ConfigureAwait(false); }
+                    catch (RequestFailedException deletionException) when (deletionException.ErrorCode == AzureErrors.LeaseIdMissing)
+                    {
+                        // Handle the race condition where we try to delete and someone else acquired it:
+                        // in that case only the original Exception from TryAcquireAsync should be thrown.
+                    }
                     catch (Exception deletionException)
                     {
                         throw new AggregateException(retryException, deletionException);
@@ -144,7 +151,7 @@ static string ConvertToValidName(string name)
                 }
             }
 
-            throw;
+            throw acquireException;
         }
 
         var shouldDeleteBlob = isRetryAfterCreate
@@ -160,7 +167,7 @@ internal sealed class InternalHandle : IDistributedSynchronizationHandle, LeaseM
         private readonly bool _ownsBlob;
         private readonly AzureBlobLeaseDistributedLock _lock;
         private readonly LeaseMonitor _leaseMonitor;
-        
+
         public InternalHandle(BlobLeaseClientWrapper leaseClient, bool ownsBlob, AzureBlobLeaseDistributedLock @lock)
         {
             this._leaseClient = leaseClient;
@@ -193,7 +200,7 @@ public async ValueTask DisposeAsync()
             {
                 await this._lock._blobClient.DeleteIfExistsAsync(leaseId: this._leaseClient.LeaseId).ConfigureAwait(false);
             }
-            else 
+            else
             {
                 await this._leaseClient.ReleaseAsync().ConfigureAwait(false);
             }

src/DistributedLock.Azure/BlobLeaseClientWrapper.cs

@@ -1,49 +1,46 @@
-using Azure.Storage.Blobs.Specialized;
+using Azure;
+using Azure.Storage.Blobs.Specialized;
 using Medallion.Threading.Internal;
 
 namespace Medallion.Threading.Azure;
 
 /// <summary>
 /// Adds <see cref="SyncViaAsync"/> support to <see cref="BlobLeaseClient"/>
 /// </summary>
-internal sealed class BlobLeaseClientWrapper
+internal sealed class BlobLeaseClientWrapper(BlobLeaseClient blobLeaseClient)
 {
-    private readonly BlobLeaseClient _blobLeaseClient;
+    public string LeaseId => blobLeaseClient.LeaseId;
 
-    public BlobLeaseClientWrapper(BlobLeaseClient blobLeaseClient)
+    public ValueTask<Response> AcquireAsync(TimeoutValue duration, CancellationToken cancellationToken)
     {
-        this._blobLeaseClient = blobLeaseClient;
-    }
-
-    public string LeaseId => this._blobLeaseClient.LeaseId;
-
-    public ValueTask AcquireAsync(TimeoutValue duration, CancellationToken cancellationToken)
-    {
-        if (SyncViaAsync.IsSynchronous)
+        RequestContext requestContext = new()
         {
-            this._blobLeaseClient.Acquire(duration.TimeSpan, cancellationToken: cancellationToken);
-            return default;
-        }
-        return new ValueTask(this._blobLeaseClient.AcquireAsync(duration.TimeSpan, cancellationToken: cancellationToken));
+            CancellationToken = cancellationToken,
+            ErrorOptions = ErrorOptions.NoThrow
+        };
+
+        return SyncViaAsync.IsSynchronous
+            ? new ValueTask<Response>(blobLeaseClient.Acquire(duration.TimeSpan, conditions: null, requestContext))
+            : new ValueTask<Response>(blobLeaseClient.AcquireAsync(duration.TimeSpan, conditions: null, requestContext));
     }
 
     public ValueTask RenewAsync(CancellationToken cancellationToken)
     {
         if (SyncViaAsync.IsSynchronous)
         {
-            this._blobLeaseClient.Renew(cancellationToken: cancellationToken);
+            blobLeaseClient.Renew(cancellationToken: cancellationToken);
             return default;
         }
-        return new ValueTask(this._blobLeaseClient.RenewAsync(cancellationToken: cancellationToken));
+        return new ValueTask(blobLeaseClient.RenewAsync(cancellationToken: cancellationToken));
     }
 
     public ValueTask ReleaseAsync()
     {
         if (SyncViaAsync.IsSynchronous)
         {
-            this._blobLeaseClient.Release();
+            blobLeaseClient.Release();
             return default;
         }
-        return new ValueTask(this._blobLeaseClient.ReleaseAsync());
+        return new ValueTask(blobLeaseClient.ReleaseAsync());
     }
-}
+}

src/DistributedLock.Azure/DistributedLock.Azure.csproj

@@ -11,7 +11,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 locking implementation based on Azure blob leases</Description>

src/DistributedLock.Core/DistributedLock.Core.csproj

@@ -11,7 +11,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.7</Version>
+    <Version>1.0.8</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Core interfaces and utilities that support the DistributedLock.* family of packages</Description>

src/DistributedLock.Core/Internal/Data/DatabaseConnection.cs

@@ -82,7 +82,12 @@ public async ValueTask OpenAsync(CancellationToken cancellationToken)
         if ((cancellationToken.CanBeCanceled || !SyncViaAsync.IsSynchronous)
             && this.InnerConnection is DbConnection dbConnection)
         {
-            await dbConnection.OpenAsync(cancellationToken).ConfigureAwait(false);
+            try { await dbConnection.OpenAsync(cancellationToken).ConfigureAwait(false); }
+            // Oracle can throw OracleException instead of OCE here
+            catch (Exception ex) when (cancellationToken.IsCancellationRequested && this.IsCommandCancellationException(ex))
+            {
+                throw new OperationCanceledException("Connection open canceled", ex, cancellationToken);
+            }
         }
         else
         {

src/DistributedLock.FileSystem/DistributedLock.FileSystem.csproj

@@ -11,7 +11,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 file locks</Description>

src/DistributedLock.FileSystem/FileDistributedLock.cs

@@ -12,7 +12,7 @@ public sealed partial class FileDistributedLock : IInternalDistributedLock<FileD
     /// before we assume that the issue is non-transient. Empirically I've found this value to be reliable both locally and on AppVeyor (if there 
     /// IS a problem there's little risk to trying more times because we'll eventually be failing hard).
     /// </summary>
-    private const int MaxUnauthorizedAccessExceptionRetries = 800;
+    private const int MaxUnauthorizedAccessExceptionRetries = 1600;
 
     // These are not configurable currently because in the future we may want to change the implementation of FileDistributedLock
     // to leverage native methods which may allow for actual blocking. The values here reflect the idea that we expect file locks

src/DistributedLock.Oracle/DistributedLock.Oracle.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFrameworks>netstandard2.1;net462</TargetFrameworks>
+    <TargetFrameworks>netstandard2.1;net472</TargetFrameworks>
     <RootNamespace>Medallion.Threading.Oracle</RootNamespace>
     <GenerateDocumentationFile>True</GenerateDocumentationFile>
     <WarningLevel>4</WarningLevel>
@@ -11,7 +11,7 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>1.0.3</Version>
+    <Version>1.0.4</Version>
     <AssemblyVersion>1.0.0.0</AssemblyVersion>
     <Authors>Michael Adelson</Authors>
     <Description>Provides a distributed lock implementation based on Oracle Database</Description>
@@ -46,7 +46,7 @@
 
   <ItemGroup>
     <PackageReference Include="Oracle.ManagedDataAccess.Core" Condition="'$(TargetFramework)' == 'netstandard2.1'" />
-    <PackageReference Include="Oracle.ManagedDataAccess" Condition="'$(TargetFramework)' == 'net462'"/>
+    <PackageReference Include="Oracle.ManagedDataAccess" Condition="'$(TargetFramework)' == 'net472'"/>
     <PackageReference Include="Microsoft.SourceLink.GitHub" PrivateAssets="All" />
     <PackageReference Include="Microsoft.CodeAnalysis.PublicApiAnalyzers" PrivateAssets="All" />
   </ItemGroup>