Merge pull request #114 from madelson/release-2.3

Release 2.3

DistributedLock.Core/AssemblyAttributes.cs

@@ -14,4 +14,5 @@
 [assembly: InternalsVisibleTo("DistributedLock.Redis, PublicKey=0024000004800000940000000602000000240000525341310004000001000100fd3af56ccc8ed94fffe25bfd651e6a5674f8f20a76d37de800dd0f7380e04f0fde2da6fa200380b14fe398605b6f470c87e5e0a0bf39ae871f07536a4994aa7a0057c4d3bcedc8fef3eecb0c88c2024a1b3289305c2393acd9fb9f9a42d0bd7826738ce864d507575ea3a1fe1746ab19823303269f79379d767949807f494be8")]
 [assembly: InternalsVisibleTo("DistributedLock.ZooKeeper, PublicKey=0024000004800000940000000602000000240000525341310004000001000100fd3af56ccc8ed94fffe25bfd651e6a5674f8f20a76d37de800dd0f7380e04f0fde2da6fa200380b14fe398605b6f470c87e5e0a0bf39ae871f07536a4994aa7a0057c4d3bcedc8fef3eecb0c88c2024a1b3289305c2393acd9fb9f9a42d0bd7826738ce864d507575ea3a1fe1746ab19823303269f79379d767949807f494be8")]
 [assembly: InternalsVisibleTo("DistributedLock.MySql, PublicKey=0024000004800000940000000602000000240000525341310004000001000100fd3af56ccc8ed94fffe25bfd651e6a5674f8f20a76d37de800dd0f7380e04f0fde2da6fa200380b14fe398605b6f470c87e5e0a0bf39ae871f07536a4994aa7a0057c4d3bcedc8fef3eecb0c88c2024a1b3289305c2393acd9fb9f9a42d0bd7826738ce864d507575ea3a1fe1746ab19823303269f79379d767949807f494be8")]
+[assembly: InternalsVisibleTo("DistributedLock.Oracle, PublicKey=0024000004800000940000000602000000240000525341310004000001000100fd3af56ccc8ed94fffe25bfd651e6a5674f8f20a76d37de800dd0f7380e04f0fde2da6fa200380b14fe398605b6f470c87e5e0a0bf39ae871f07536a4994aa7a0057c4d3bcedc8fef3eecb0c88c2024a1b3289305c2393acd9fb9f9a42d0bd7826738ce864d507575ea3a1fe1746ab19823303269f79379d767949807f494be8")]
 #endif

DistributedLock.Core/DistributedLock.Core.csproj

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

DistributedLock.Core/Internal/Data/DatabaseCommand.cs

@@ -159,7 +159,9 @@ private async ValueTask<TResult> InternalExecuteAndPropagateCancellationAsync<TS
             Invariant.Require(cancellationToken.CanBeCanceled);
 
             using var _ = await this.AcquireConnectionLockIfNeeded(isConnectionMonitoringQuery).ConfigureAwait(false);
-            await this.PrepareIfNeededAsync(cancellationToken).ConfigureAwait(false);
+            // Note: for now we cannot pass cancellationToken to PrepareAsync() because this will break on Postgres which
+            // is the only db we currently support that needs Prepare currently. See https://github.com/npgsql/npgsql/issues/4209
+            await this.PrepareIfNeededAsync(CancellationToken.None).ConfigureAwait(false);
             try
             {
                 return await executeAsync(state, cancellationToken).ConfigureAwait(false);

DistributedLock.FileSystem/DistributedLock.FileSystem.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 file locks</Description>

DistributedLock.FileSystem/FileDistributedLock.cs

@@ -13,6 +13,13 @@ namespace Medallion.Threading.FileSystem
     /// </summary>
     public sealed partial class FileDistributedLock : IInternalDistributedLock<FileDistributedLockHandle>
     {
+        /// <summary>
+        /// Since <see cref="UnauthorizedAccessException"/> can be thrown EITHER transiently or for permissions issues, we retry up to this many times
+        /// 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 = 400;
+
         // 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
         // to be used in cases where contention is rare
@@ -64,15 +71,13 @@ public FileDistributedLock(DirectoryInfo lockFileDirectory, string name)
 
         private FileDistributedLockHandle? TryAcquire(CancellationToken cancellationToken)
         {
+            var retryCount = 0;
+
             while (true)
             {
                 cancellationToken.ThrowIfCancellationRequested();
 
-                try { System.IO.Directory.CreateDirectory(this.Directory); }
-                catch (Exception ex)
-                {
-                    throw new InvalidOperationException($"Failed to ensure that lock file directory {this.Directory} exists", ex);
-                }
+                this.EnsureDirectoryExists();
 
                 FileStream lockFileStream;
                 try
@@ -88,9 +93,36 @@ public FileDistributedLock(DirectoryInfo lockFileDirectory, string name)
                     // this should almost never happen because we just created the directory but in a race condition it could. Just retry
                     continue;
                 }
-                catch (UnauthorizedAccessException) when (System.IO.Directory.Exists(this.Name))
+                catch (UnauthorizedAccessException)
                 {
-                    throw new InvalidOperationException($"Failed to create lock file '{this.Name}' because it is already the name of a directory");
+                    // This can happen in few cases:
+
+                    // The path is already directory, so we'll never be able to open a handle of it as a file
+                    if (System.IO.Directory.Exists(this.Name))
+                    {
+                        throw new InvalidOperationException($"Failed to create lock file '{this.Name}' because it is already the name of a directory");
+                    }
+
+                    // The file exists and is read-only
+                    FileAttributes attributes;
+                    try { attributes = File.GetAttributes(this.Name); }
+                    catch { attributes = FileAttributes.Normal; } // e. g. could fail with FileNotFoundException
+                    if (attributes.HasFlag(FileAttributes.ReadOnly))
+                    {
+                        // We could support this by eschewing DeleteOnClose once we detect that a file is read-only,
+                        // but absent interest or a use-case we'll just throw for now
+                        throw new NotSupportedException($"Locking on read-only file '{this.Name}' is not supported");
+                    }
+
+                    // Frustratingly, this error can be thrown transiently due to concurrent creation/deletion. Initially assume
+                    // that it is transient and just retry
+                    if (++retryCount <= MaxUnauthorizedAccessExceptionRetries)
+                    {
+                        continue;
+                    }
+
+                    // If we get here, we've exhausted our retries: assume that it is a legitimate permissions issue
+                    throw;
                 }
                 // this should never happen because we validate. However if it does (e. g. due to some system configuration change?), throw so that
                 // this doesn't end up in the IOException block (PathTooLongException is IOException)
@@ -104,5 +136,26 @@ public FileDistributedLock(DirectoryInfo lockFileDirectory, string name)
                 return new FileDistributedLockHandle(lockFileStream);
             }
         }
+
+        private void EnsureDirectoryExists()
+        {
+            var retryCount = 0;
+
+            while (true)
+            {
+                try
+                {
+                    System.IO.Directory.CreateDirectory(this.Directory);
+                    return;
+                }
+                // This can indicate either a transient failure during concurrent creation/deletion or a permissions issue.
+                // If we encounter it, assume it is transient unless it persists.
+                catch (UnauthorizedAccessException) when (++retryCount <= MaxUnauthorizedAccessExceptionRetries) { }
+                catch (Exception ex)
+                {
+                    throw new InvalidOperationException($"Failed to ensure that lock file directory {this.Directory} exists", ex);
+                }
+            }
+        }
     }
 }

DistributedLock.MySql/MySqlDistributedLock.cs

@@ -164,14 +164,14 @@ private static IDbDistributedLock CreateInternalLock(string name, string connect
             return new DedicatedConnectionOrTransactionDbDistributedLock(name, () => new MySqlDatabaseConnection(connectionString), useTransaction: false, keepaliveCadence);
         }
 
-        static IDbDistributedLock CreateInternalLock(string name, IDbConnection connection)
+        private static IDbDistributedLock CreateInternalLock(string name, IDbConnection connection)
         {
             if (connection == null) { throw new ArgumentNullException(nameof(connection)); }
 
             return new DedicatedConnectionOrTransactionDbDistributedLock(name, () => new MySqlDatabaseConnection(connection));
         }
 
-        static IDbDistributedLock CreateInternalLock(string name, IDbTransaction transaction)
+        private static IDbDistributedLock CreateInternalLock(string name, IDbTransaction transaction)
         {
             if (transaction == null) { throw new ArgumentNullException(nameof(transaction)); }
 

DistributedLock.Oracle/AssemblyAttributes.cs

@@ -0,0 +1,3 @@
+using System.Runtime.CompilerServices;
+
+[assembly: InternalsVisibleTo("DistributedLock.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100fd3af56ccc8ed94fffe25bfd651e6a5674f8f20a76d37de800dd0f7380e04f0fde2da6fa200380b14fe398605b6f470c87e5e0a0bf39ae871f07536a4994aa7a0057c4d3bcedc8fef3eecb0c88c2024a1b3289305c2393acd9fb9f9a42d0bd7826738ce864d507575ea3a1fe1746ab19823303269f79379d767949807f494be8")]

DistributedLock.Oracle/DistributedLock.Oracle.csproj

@@ -0,0 +1,55 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFrameworks>netstandard2.1;net462</TargetFrameworks>
+    <RootNamespace>Medallion.Threading.Oracle</RootNamespace>
+    <GenerateDocumentationFile>True</GenerateDocumentationFile>
+    <WarningLevel>4</WarningLevel>
+    <LangVersion>Latest</LangVersion>
+    <Nullable>enable</Nullable>
+  </PropertyGroup>
+
+  <PropertyGroup>
+    <Version>1.0.0</Version>
+    <AssemblyVersion>1.0.0.0</AssemblyVersion>
+    <Authors>Michael Adelson</Authors>
+    <Description>Provides a distributed lock implementation based on Oracle Database</Description>
+    <Copyright>Copyright © 2021 Michael Adelson</Copyright>
+    <PackageLicenseExpression>MIT</PackageLicenseExpression>
+    <PackageTags>distributed lock async mutex reader writer sql oracle</PackageTags>
+    <PackageProjectUrl>https://github.com/madelson/DistributedLock</PackageProjectUrl>
+    <RepositoryUrl>https://github.com/madelson/DistributedLock</RepositoryUrl>
+    <FileVersion>1.0.0.0</FileVersion>
+    <PackageReleaseNotes>See https://github.com/madelson/DistributedLock#release-notes</PackageReleaseNotes>
+    <SignAssembly>true</SignAssembly>
+    <AssemblyOriginatorKeyFile>..\DistributedLock.snk</AssemblyOriginatorKeyFile>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)' == 'Release'">
+    <Optimize>True</Optimize>
+    <GeneratePackageOnBuild>True</GeneratePackageOnBuild>
+    <TreatWarningsAsErrors>True</TreatWarningsAsErrors>
+    <TreatSpecificWarningsAsErrors />
+    <!-- see https://github.com/dotnet/sdk/issues/2679 -->
+    <DebugType>embedded</DebugType>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(Configuration)' == 'Debug'">
+    <Optimize>False</Optimize>
+    <NoWarn>1591</NoWarn>
+    <DefineConstants>TRACE;DEBUG</DefineConstants>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Oracle.ManagedDataAccess.Core" Version="3.21.4" Condition="'$(TargetFramework)' == 'netstandard2.1'" />
+    <PackageReference Include="Oracle.ManagedDataAccess" Version="21.4.0" Condition="'$(TargetFramework)' == 'net462'"/>
+    <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\DistributedLock.Core\DistributedLock.Core.csproj" />
+  </ItemGroup>
+
+  <Import Project="..\CopyPackageToPublishDirectory.targets" />
+  <Import Project="..\FixDistributedLockCoreDependencyVersion.targets" />
+</Project>

DistributedLock.Oracle/OracleConnectionOptionsBuilder.cs

@@ -0,0 +1,74 @@
+using Medallion.Threading.Internal;
+using System;
+using System.Collections.Generic;
+using System.Text;
+using System.Threading;
+
+namespace Medallion.Threading.Oracle
+{
+    /// <summary>
+    /// Specifies options for connecting to and locking against an Oracle database
+    /// </summary>
+    public sealed class OracleConnectionOptionsBuilder
+    {
+        private TimeoutValue? _keepaliveCadence;
+        private bool? _useMultiplexing;
+
+        internal OracleConnectionOptionsBuilder() { }
+
+        /// <summary>
+        /// Oracle does not kill idle connections by default, so by default keepalive is disabled (set to <see cref="Timeout.InfiniteTimeSpan"/>).
+        /// 
+        /// However, if you are using the IDLE_TIME setting in Oracle or if your network is dropping connections that are idle holding locks for
+        /// a long time, you can set a value for keepalive to prevent this from happening.
+        /// 
+        /// See https://stackoverflow.com/questions/1966247/idle-timeout-parameter-in-oracle.
+        /// </summary>
+        public OracleConnectionOptionsBuilder KeepaliveCadence(TimeSpan keepaliveCadence)
+        {
+            this._keepaliveCadence = new TimeoutValue(keepaliveCadence, nameof(keepaliveCadence));
+            return this;
+        }
+
+        /// <summary>
+        /// This mode takes advantage of the fact that while "holding" a lock (or other synchronization primitive)
+        /// a connection is essentially idle. Thus, rather than creating a new connection for each held lock it is 
+        /// often possible to multiplex a shared connection so that that connection can hold multiple locks at the same time.
+        /// 
+        /// Multiplexing is on by default.
+        /// 
+        /// This is implemented in such a way that releasing a lock held on such a connection will never be blocked by an
+        /// Acquire() call that is waiting to acquire a lock on that same connection. For this reason, the multiplexing
+        /// strategy is "optimistic": if the lock can't be acquired instantaneously on the shared connection, a new (shareable) 
+        /// connection will be allocated.
+        /// 
+        /// This option can improve performance and avoid connection pool starvation in high-load scenarios. It is also
+        /// particularly applicable to cases where <see cref="IDistributedLock.TryAcquire(TimeSpan, System.Threading.CancellationToken)"/>
+        /// semantics are used with a zero-length timeout.
+        /// </summary>
+        public OracleConnectionOptionsBuilder UseMultiplexing(bool useMultiplexing = true)
+        {
+            this._useMultiplexing = useMultiplexing;
+            return this;
+        }
+
+        internal static (TimeoutValue keepaliveCadence, bool useMultiplexing) GetOptions(Action<OracleConnectionOptionsBuilder>? optionsBuilder)
+        {
+            OracleConnectionOptionsBuilder? options;
+            if (optionsBuilder != null)
+            {
+                options = new OracleConnectionOptionsBuilder();
+                optionsBuilder(options);
+            }
+            else
+            {
+                options = null;
+            }
+
+            var keepaliveCadence = options?._keepaliveCadence ?? Timeout.InfiniteTimeSpan;
+            var useMultiplexing = options?._useMultiplexing ?? true;
+
+            return (keepaliveCadence, useMultiplexing);
+        }
+    }
+}

DistributedLock.Oracle/OracleDatabaseConnection.cs

@@ -0,0 +1,89 @@
+using Medallion.Threading.Internal.Data;
+using Oracle.ManagedDataAccess.Client;
+using System;
+using System.Collections.Generic;
+using System.Data;
+using System.Linq;
+using System.Text;
+using System.Threading;
+using System.Threading.Tasks;
+
+namespace Medallion.Threading.Oracle
+{
+    internal class OracleDatabaseConnection : DatabaseConnection
+    {
+        public const string ApplicationNameIndicatorPrefix = "__DistributedLock.ApplicationName=";
+
+        // see SleepAsync() for why we need this
+        private readonly IDbConnection _innerConnection;
+
+        public OracleDatabaseConnection(IDbConnection connection)
+            : this(connection, isExternallyOwned: true)
+        {
+        }
+
+        public OracleDatabaseConnection(IDbTransaction transaction)
+            : base(transaction, isExternallyOwned: true)
+        {
+            this._innerConnection = transaction.Connection;
+        }
+
+        public OracleDatabaseConnection(string connectionString)
+            : this(CreateConnection(connectionString), isExternallyOwned: false)
+        {
+        }
+
+        private OracleDatabaseConnection(IDbConnection connection, bool isExternallyOwned)
+            : base(connection, isExternallyOwned)
+        {
+            this._innerConnection = connection;
+        }
+
+        // from https://docs.oracle.com/html/E10927_01/OracleCommandClass.htm "this method is a no-op" wrt "Prepare()"
+        public override bool ShouldPrepareCommands => false;
+
+        public override bool IsCommandCancellationException(Exception exception) => 
+            exception is OracleException oracleException
+                // based on https://docs.oracle.com/cd/E85694_01/ODPNT/CommandCancel.htm
+                && (oracleException.Number == 01013 || oracleException.Number == 00936 || oracleException.Number == 00604);
+
+        public override async Task SleepAsync(TimeSpan sleepTime, CancellationToken cancellationToken, Func<DatabaseCommand, CancellationToken, ValueTask<int>> executor)
+        {
+            using var sleepCommand = this.CreateCommand();
+            sleepCommand.SetCommandText("BEGIN sys.DBMS_SESSION.SLEEP(:seconds) END;");
+            sleepCommand.AddParameter("seconds", sleepTime.TotalSeconds);
+
+            try
+            {
+                await executor(sleepCommand, cancellationToken).ConfigureAwait(false);
+            }
+            catch when (!cancellationToken.IsCancellationRequested)
+            {
+                // Oracle doesn't fire StateChange unless the State is observed or the connection is explicitly opened/closed. Therefore, we observe
+                // the state on seeing any exception in order to for the event to fire. See https://github.com/oracle/dotnet-db-samples/issues/226
+                _ = this._innerConnection.State;
+                throw;
+            }
+        }
+
+        public static OracleConnection CreateConnection(string connectionString)
+        {
+            if (connectionString == null) { throw new ArgumentNullException(connectionString, nameof(connectionString)); }
+
+            // The .NET Oracle provider does not currently support ApplicationName natively as a connection string property.
+            // However, that functionality is relied on by many of our tests. As a workaround, we permit the application name
+            // to be included in the connection string using a custom encoding scheme. This is only intended to work in tests!
+            // See https://github.com/oracle/dotnet-db-samples/issues/216 for more context.
+            if (connectionString.StartsWith(ApplicationNameIndicatorPrefix, StringComparison.Ordinal))
+            {
+                var firstSeparatorIndex = connectionString.IndexOf(';');
+                var applicationName = connectionString.Substring(startIndex: ApplicationNameIndicatorPrefix.Length, length: firstSeparatorIndex - ApplicationNameIndicatorPrefix.Length);
+                var connection = new OracleConnection(connectionString.Substring(startIndex: firstSeparatorIndex + 1));
+                connection.ConnectionOpen += _ => connection.ClientInfo = applicationName;
+                return connection;
+            }
+
+            return new OracleConnection(connectionString);
+        }
+    }
+}