diff --git a/source/Calamari.Common/Features/Deployment/Journal/DeploymentJournal.cs b/source/Calamari.Common/Features/Deployment/Journal/DeploymentJournal.cs index 2c6bd8584d..18d4e28e20 100644 --- a/source/Calamari.Common/Features/Deployment/Journal/DeploymentJournal.cs +++ b/source/Calamari.Common/Features/Deployment/Journal/DeploymentJournal.cs @@ -3,7 +3,7 @@ using System.IO; using System.Linq; using System.Xml.Linq; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Logging; using Calamari.Common.Plumbing.Variables; @@ -12,16 +12,16 @@ namespace Calamari.Common.Features.Deployment.Journal { public class DeploymentJournal : IDeploymentJournal { - const string SemaphoreName = "Octopus.Calamari.DeploymentJournal"; + const string NamedLockName = "Octopus.Calamari.DeploymentJournal"; readonly ICalamariFileSystem fileSystem; - readonly ISemaphoreFactory semaphore; + readonly INamedLockManager namedLockManager; readonly IVariables variables; readonly ILog log; - public DeploymentJournal(ICalamariFileSystem fileSystem, ISemaphoreFactory semaphore, IVariables variables, ILog log) + public DeploymentJournal(ICalamariFileSystem fileSystem, INamedLockManager namedLockManager, IVariables variables, ILog log) { this.fileSystem = fileSystem; - this.semaphore = semaphore; + this.namedLockManager = namedLockManager; this.variables = variables; this.log = log; } @@ -30,7 +30,7 @@ public DeploymentJournal(ICalamariFileSystem fileSystem, ISemaphoreFactory semap internal void AddJournalEntry(JournalEntry entry) { - using (semaphore.Acquire(SemaphoreName, "Another process is using the deployment journal")) + using (namedLockManager.Acquire(NamedLockName, "Another process is using the deployment journal")) { var xElement = entry.ToXmlElement(); log.VerboseFormat("Adding journal entry:\n{0}", xElement.ToString()); @@ -40,7 +40,7 @@ internal void AddJournalEntry(JournalEntry entry) public List GetAllJournalEntries() { - using (semaphore.Acquire(SemaphoreName, "Another process is using the deployment journal")) + using (namedLockManager.Acquire(NamedLockName, "Another process is using the deployment journal")) { return Read().Select(element => new JournalEntry(element)).ToList(); } @@ -48,7 +48,7 @@ public List GetAllJournalEntries() public void RemoveJournalEntries(IEnumerable ids) { - using (semaphore.Acquire(SemaphoreName, "Another process is using the deployment journal")) + using (namedLockManager.Acquire(NamedLockName, "Another process is using the deployment journal")) { var elements = Read(); diff --git a/source/Calamari.Common/Features/Processes/Semaphores/ISemaphoreFactory.cs b/source/Calamari.Common/Features/Processes/NamedLocks/INamedLockManager.cs similarity index 50% rename from source/Calamari.Common/Features/Processes/Semaphores/ISemaphoreFactory.cs rename to source/Calamari.Common/Features/Processes/NamedLocks/INamedLockManager.cs index 1240f3d196..3d5491a9c3 100644 --- a/source/Calamari.Common/Features/Processes/Semaphores/ISemaphoreFactory.cs +++ b/source/Calamari.Common/Features/Processes/NamedLocks/INamedLockManager.cs @@ -1,8 +1,8 @@ using System; -namespace Calamari.Common.Features.Processes.Semaphores +namespace Calamari.Common.Features.Processes.NamedLocks { - public interface ISemaphoreFactory + public interface INamedLockManager { IDisposable Acquire(string name, string waitMessage); } diff --git a/source/Calamari.Common/Features/Processes/NamedLocks/MutexBasedNamedLockManager.cs b/source/Calamari.Common/Features/Processes/NamedLocks/MutexBasedNamedLockManager.cs new file mode 100644 index 0000000000..d0f30c1424 --- /dev/null +++ b/source/Calamari.Common/Features/Processes/NamedLocks/MutexBasedNamedLockManager.cs @@ -0,0 +1,193 @@ +using System; +using System.Runtime.ExceptionServices; +using System.Runtime.Versioning; +using System.Security.AccessControl; +using System.Security.Principal; +using System.Threading; +using Calamari.Common.Plumbing.Extensions; +using Calamari.Common.Plumbing.Logging; +using Polly; +using Polly.Retry; + +namespace Calamari.Common.Features.Processes.NamedLocks +{ + public class MutexBasedNamedLockManager : INamedLockManager + { + readonly ILog log; + readonly int initialWaitBeforeShowingLogMessage; + readonly ResiliencePipeline mutexCreationPipeline; + + public MutexBasedNamedLockManager() + { + log = ConsoleLog.Instance; + initialWaitBeforeShowingLogMessage = (int)TimeSpan.FromSeconds(3).TotalMilliseconds; + + mutexCreationPipeline = new ResiliencePipelineBuilder() + .AddRetry(new RetryStrategyOptions() + { + ShouldHandle = new PredicateBuilder().Handle(), + MaxRetryAttempts = 5, //means we'll wait for a max of around 250ms + BackoffType = DelayBackoffType.Linear, + UseJitter = true, + Delay = TimeSpan.FromMilliseconds(50), + OnRetry = args => + { + log.Verbose($"Waiting {args.RetryDelay.TotalMilliseconds}ms before attempting to create the Mutex again"); + return default; + } + }) + .Build(); + } + + public IDisposable Acquire(string name, string waitMessage) + { + log.Verbose($"Acquiring named lock for {name}"); + + var trackedThread = new TrackedThread($"Mutex owner for '{name}'", + tracker => + { + var globalName = $@"Global\{name}"; + + Mutex? mutex = null; + try + { + // Create/acquire the global mutex with some retry, to (hopefully) avoid two instances of + // Calamari racing to create it (e.g. parallel steps on the same machine) + mutex = mutexCreationPipeline.Execute(() => new Mutex(false, globalName)); + + + // Assign full control for all users, so that a lock taken by (say) a Tentacle running as a service + // is still accessible to Calamari running under a different account + if (OperatingSystem.IsWindows()) + { + SetFullAccessControlForAllUsers(mutex, globalName); + log.Verbose($"Calamari mutex configured to allow control for all users on '{name}'"); + } + + try + { + log.Verbose($"Attempting to acquire mutex on Calamari '{name}'"); + if (!mutex.WaitOne(initialWaitBeforeShowingLogMessage)) + { + log.Verbose(waitMessage); + mutex.WaitOne(); + } + } + catch (AbandonedMutexException) + { + // The previous owner died without releasing; the kernel has handed ownership to us + log.Warn($"The mutex '{name}' was abandoned by a previous process that exited without releasing it. Continuing, but anything it was protecting may have been left in an inconsistent state."); + } + } + catch (Exception ex) + { + // We never took the mutex, so there is nothing to release - just + // close the handle rather than leaving it to the finaliser + mutex?.Dispose(); + tracker.MarkAsErrored(ex); + return; + } + + log.Verbose($"Acquired lock on Calamari for '{name}'"); + tracker.HoldUntilDisposed(); + + // An unhandled exception here would terminate the process. Releasing is best effort: + // a failure leaves the mutex abandoned, which the next waiter recovers from. + try + { + try + { + mutex.ReleaseMutex(); + } + finally + { + mutex.Dispose(); + } + } + catch (Exception ex) + { + log.Verbose($"Failed to release the mutex '{globalName}': {ex.PrettyPrint()}"); + } + }); + + trackedThread.StartAndBlockUntilLockAcquired(); + return trackedThread; + } + + [SupportedOSPlatform("windows")] + void SetFullAccessControlForAllUsers(Mutex mutex, string name) + { + var mutexSecurity = new MutexSecurity(); + var everyone = new SecurityIdentifier(WellKnownSidType.WorldSid, null); + var rule = new MutexAccessRule(everyone, MutexRights.FullControl, AccessControlType.Allow); + + mutexSecurity.AddAccessRule(rule); + + try + { + mutex.SetAccessControl(mutexSecurity); + } + catch (Exception e) + { + log.Verbose($"Failed to set access controls on mutex '{name}': {e.PrettyPrint()}"); + } + } + + class TrackedThread : IDisposable + { + readonly ManualResetEventSlim acquired = new(false); + readonly ManualResetEventSlim release = new(false); + readonly Thread owner; + + int released; + Exception? error; + + public TrackedThread(string name, Action threadStart) + { + owner = new Thread(() => threadStart(this)) + { + IsBackground = true, + Name = name + }; + } + + public void StartAndBlockUntilLockAcquired() + { + owner.Start(); + + // Wait for the owner thread to signal it has "entered", or errored + acquired.Wait(); + + if (error != null) + { + Dispose(); + ExceptionDispatchInfo.Capture(error).Throw(); + } + } + + public void HoldUntilDisposed() + { + acquired.Set(); + release.Wait(); + } + + public void MarkAsErrored(Exception ex) + { + error = ex; + acquired.Set(); + } + + public void Dispose() + { + // The events are gone after the first time through, so guard against a caller disposing twice + if (Interlocked.Exchange(ref released, 1) != 0) + return; + + release.Set(); + owner.Join(); + acquired.Dispose(); + release.Dispose(); + } + } + } +} diff --git a/source/Calamari.Common/Features/Processes/Semaphores/SystemSemaphoreManager.cs b/source/Calamari.Common/Features/Processes/Semaphores/SystemSemaphoreManager.cs deleted file mode 100644 index 0e232ffd75..0000000000 --- a/source/Calamari.Common/Features/Processes/Semaphores/SystemSemaphoreManager.cs +++ /dev/null @@ -1,145 +0,0 @@ -using System; -using System.Runtime.Versioning; -using System.Security.AccessControl; -using System.Security.Principal; -using System.Threading; -using Calamari.Common.Plumbing; -using Calamari.Common.Plumbing.Extensions; -using Calamari.Common.Plumbing.Logging; -using Polly; -using Polly.Retry; - -namespace Calamari.Common.Features.Processes.Semaphores -{ - public class SystemSemaphoreManager : ISemaphoreFactory - { - readonly ILog log; - readonly int initialWaitBeforeShowingLogMessage; - readonly ResiliencePipeline semaphoreAcquisitionPipeline; - - public SystemSemaphoreManager() - { - log = ConsoleLog.Instance; - initialWaitBeforeShowingLogMessage = (int)TimeSpan.FromSeconds(3).TotalMilliseconds; - - semaphoreAcquisitionPipeline = new ResiliencePipelineBuilder() - .AddRetry(new RetryStrategyOptions() - { - ShouldHandle = new PredicateBuilder().Handle(), - MaxRetryAttempts = 5, //means we'll wait for a max of around 250ms - BackoffType = DelayBackoffType.Linear, - UseJitter = true, - Delay = TimeSpan.FromMilliseconds(50), - OnRetry = args => - { - log.Verbose($"Waiting {args.RetryDelay.TotalMilliseconds}ms before attempting to acquire the Semaphore again"); - return default; - } - }) - .Build(); - } - - public IDisposable Acquire(string name, string waitMessage) - { - return OperatingSystem.IsWindows() - ? AcquireSemaphore(name, waitMessage) - : AcquireMutex(name, waitMessage); - } - - [SupportedOSPlatform("windows")] - IDisposable AcquireSemaphore(string name, string waitMessage) - { - var globalName = $"Global\\{name}"; - - //we try and create/acquire a global semaphore with some retry - //this is done to (hopefully) avoid situations where two instances of Calamari are trying to acquire the same semaphore - //this could happen in the case of parallel steps being executed on the same machine - var semaphore = semaphoreAcquisitionPipeline.Execute(() => new Semaphore(1,1, name)); - - //assign full control for all use - SetFullAccessControlForAllUsers(semaphore, globalName); - - try - { - if (!semaphore.WaitOne(initialWaitBeforeShowingLogMessage)) - { - log.Verbose(waitMessage); - semaphore.WaitOne(); - } - } - catch (AbandonedMutexException) - { - // We are now the owners of the mutex - // If a thread terminates while owning a mutex, the mutex is said to be abandoned. - // The state of the mutex is set to signaled and the next waiting thread gets ownership. - } - - return new Releaser(() => - { - semaphore.Release(); - semaphore.Dispose(); - }); - } - - IDisposable AcquireMutex(string name, string waitMessage) - { - var globalName = $"Global\\{name}"; - var mutex = new Mutex(false, globalName); - - try - { - if (!mutex.WaitOne(initialWaitBeforeShowingLogMessage)) - { - log.Verbose(waitMessage); - mutex.WaitOne(); - } - } - catch (AbandonedMutexException) - { - // We are now the owners of the mutex - // If a thread terminates while owning a mutex, the mutex is said to be abandoned. - // The state of the mutex is set to signaled and the next waiting thread gets ownership. - } - - return new Releaser(() => - { - mutex.ReleaseMutex(); - mutex.Dispose(); - }); - } - - [SupportedOSPlatform("windows")] - void SetFullAccessControlForAllUsers(Semaphore semaphore, string name) - { - var semaphoreSecurity = new SemaphoreSecurity(); - var everyone = new SecurityIdentifier(WellKnownSidType.WorldSid, null); - var rule = new SemaphoreAccessRule(everyone, SemaphoreRights.FullControl, AccessControlType.Allow); - - semaphoreSecurity.AddAccessRule(rule); - - try - { - semaphore.SetAccessControl(semaphoreSecurity); - } - catch (Exception e) - { - log.Verbose($"Failed to set access controls on semaphore '{name}': {e.PrettyPrint()}"); - } - } - - class Releaser : IDisposable - { - readonly Action dispose; - - public Releaser(Action dispose) - { - this.dispose = dispose; - } - - public void Dispose() - { - dispose(); - } - } - } -} diff --git a/source/Calamari.Common/Features/Scripting/ScriptExecutor.cs b/source/Calamari.Common/Features/Scripting/ScriptExecutor.cs index 0994d63c08..4687c78f6a 100644 --- a/source/Calamari.Common/Features/Scripting/ScriptExecutor.cs +++ b/source/Calamari.Common/Features/Scripting/ScriptExecutor.cs @@ -2,7 +2,7 @@ using System.Collections.Generic; using System.IO; using Calamari.Common.Features.Processes; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Logging; using Calamari.Common.Plumbing.Proxies; @@ -42,7 +42,7 @@ public CommandResult Execute(Script script, try { if (execution.CommandLineInvocation.Isolate) - using (new SystemSemaphoreManager() + using (new MutexBasedNamedLockManager() .Acquire("CalamariSynchronizeProcess", "Waiting for other process to finish executing script")) { diff --git a/source/Calamari.Common/Plumbing/Deployment/Journal/DeploymentJournalWriter.cs b/source/Calamari.Common/Plumbing/Deployment/Journal/DeploymentJournalWriter.cs index 45e6fac757..a2ddedafe9 100644 --- a/source/Calamari.Common/Plumbing/Deployment/Journal/DeploymentJournalWriter.cs +++ b/source/Calamari.Common/Plumbing/Deployment/Journal/DeploymentJournalWriter.cs @@ -2,7 +2,7 @@ using System.Linq; using Calamari.Common.Commands; using Calamari.Common.Features.Deployment.Journal; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Logging; using Calamari.Common.Plumbing.Variables; @@ -31,8 +31,8 @@ public void AddJournalEntry(RunningDeployment deployment, bool wasSuccessful, st { if (deployment.SkipJournal) return; - var semaphore = new SystemSemaphoreManager(); - var journal = new DeploymentJournal(fileSystem, semaphore, deployment.Variables, log); + var namedLockManager = new MutexBasedNamedLockManager(); + var journal = new DeploymentJournal(fileSystem, namedLockManager, deployment.Variables, log); var hasPackages = !string.IsNullOrWhiteSpace(packageFile) || deployment.Variables.GetIndexes(PackageVariables.PackageCollection).Any(); diff --git a/source/Calamari.Common/Plumbing/Extensions/ApplicationDirectory.cs b/source/Calamari.Common/Plumbing/Extensions/ApplicationDirectory.cs index 3cd256b657..b277aeba86 100644 --- a/source/Calamari.Common/Plumbing/Extensions/ApplicationDirectory.cs +++ b/source/Calamari.Common/Plumbing/Extensions/ApplicationDirectory.cs @@ -1,7 +1,7 @@ using System; using System.IO; using Calamari.Common.Features.Packages; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Variables; @@ -9,7 +9,7 @@ namespace Calamari.Common.Plumbing.Extensions { public class ApplicationDirectory { - static readonly ISemaphoreFactory Semaphore = new SystemSemaphoreManager(); + static readonly INamedLockManager NamedLock = new MutexBasedNamedLockManager(); /// /// Returns the directory where the package will be installed. @@ -90,7 +90,7 @@ static string EnsureTargetPathExistsAndIsEmpty(string desiredTargetPath, ICalama { var target = desiredTargetPath; - using (Semaphore.Acquire("Octopus.Calamari.ExtractionDirectory", "Another process is finding an extraction directory, please wait...")) + using (NamedLock.Acquire("Octopus.Calamari.ExtractionDirectory", "Another process is finding an extraction directory, please wait...")) { for (var i = 1; fileSystem.DirectoryExists(target) || fileSystem.FileExists(target); i++) target = desiredTargetPath + "_" + i; diff --git a/source/Calamari.Common/Plumbing/Variables/DeploymentJournalVariableContributor.cs b/source/Calamari.Common/Plumbing/Variables/DeploymentJournalVariableContributor.cs index f9718bb7d1..95dc12a0bb 100644 --- a/source/Calamari.Common/Plumbing/Variables/DeploymentJournalVariableContributor.cs +++ b/source/Calamari.Common/Plumbing/Variables/DeploymentJournalVariableContributor.cs @@ -1,7 +1,7 @@ using System; using System.Linq; using Calamari.Common.Features.Deployment.Journal; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Logging; @@ -15,7 +15,7 @@ public static void Contribute(ICalamariFileSystem fileSystem, IVariables variabl if (string.IsNullOrWhiteSpace(policySet)) return; - var journal = new DeploymentJournal(fileSystem, new SystemSemaphoreManager(), variables, log); + var journal = new DeploymentJournal(fileSystem, new MutexBasedNamedLockManager(), variables, log); Previous(variables, journal, policySet); PreviousSuccessful(variables, journal, policySet); } diff --git a/source/Calamari.Shared/Integration/Certificates/WindowsX509CertificateStore.cs b/source/Calamari.Shared/Integration/Certificates/WindowsX509CertificateStore.cs index a0f2103901..b1dcf3bfb2 100644 --- a/source/Calamari.Shared/Integration/Certificates/WindowsX509CertificateStore.cs +++ b/source/Calamari.Shared/Integration/Certificates/WindowsX509CertificateStore.cs @@ -10,7 +10,7 @@ using System.Security.Cryptography; using System.Security.Cryptography.X509Certificates; using System.Security.Principal; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.Logging; using Calamari.Integration.Certificates.WindowsNative; using Org.BouncyCastle.Pkcs; @@ -24,8 +24,8 @@ namespace Calamari.Integration.Certificates public class WindowsX509CertificateStore : IWindowsX509CertificateStore { readonly ILog log; - public static readonly ISemaphoreFactory Semaphores = new SystemSemaphoreManager(); - public static readonly string SemaphoreName = nameof(WindowsX509CertificateStore); + public static readonly INamedLockManager NamedLocks = new MutexBasedNamedLockManager(); + public static readonly string NamedLockName = nameof(WindowsX509CertificateStore); const string IntermediateAuthorityStoreName = "CA"; public static readonly string RootAuthorityStoreName = "Root"; @@ -41,9 +41,9 @@ public WindowsX509CertificateStore(): this(ConsoleLog.Instance) } - private static IDisposable AcquireSemaphore() + private static IDisposable AcquireNamedLock() { - return Semaphores.Acquire(SemaphoreName, "Another process is working with the certificate store, please wait..."); + return NamedLocks.Acquire(NamedLockName, "Another process is working with the certificate store, please wait..."); } public string? FindCertificateStore(string thumbprint, StoreLocation storeLocation) @@ -67,7 +67,7 @@ private static IDisposable AcquireSemaphore() public void ImportCertificateToStore(byte[] pfxBytes, string password, StoreLocation storeLocation, string storeName, bool privateKeyExportable) { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { CertificateSystemStoreLocation systemStoreLocation; bool useUserKeyStore; @@ -96,7 +96,7 @@ public void ImportCertificateToStore(byte[] pfxBytes, string password, StoreLoca /// public void ImportCertificateToStore(byte[] pfxBytes, string password, string userName, string storeName, bool privateKeyExportable) { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { var account = new NTAccount(userName); var sid = (SecurityIdentifier) account.Translate(typeof(SecurityIdentifier)); @@ -125,7 +125,7 @@ public void AddPrivateKeyAccessRules(string thumbprint, StoreLocation storeLocat public void AddPrivateKeyAccessRules(string thumbprint, StoreLocation storeLocation, string storeName, ICollection privateKeyAccessRules) { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { var store = new X509Store(storeName, storeLocation); store.Open(OpenFlags.ReadWrite); @@ -152,7 +152,7 @@ public void AddPrivateKeyAccessRules(string thumbprint, StoreLocation storeLocat /// public void RemoveCertificateFromStore(string thumbprint, StoreLocation storeLocation, string storeName) { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { var store = new X509Store(storeName, storeLocation); store.Open(OpenFlags.ReadWrite); diff --git a/source/Calamari.Tests/Fixtures/Deployment/CleanFixture.cs b/source/Calamari.Tests/Fixtures/Deployment/CleanFixture.cs index 56e8e15a09..df4c55697a 100644 --- a/source/Calamari.Tests/Fixtures/Deployment/CleanFixture.cs +++ b/source/Calamari.Tests/Fixtures/Deployment/CleanFixture.cs @@ -3,7 +3,7 @@ using System.IO; using System.Xml.Linq; using Calamari.Common.Features.Deployment.Journal; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.Commands; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Variables; @@ -41,7 +41,7 @@ public void SetUp() variables = new VariablesFactory(fileSystem, new SilentLog()).Create(new CommonOptions("test")); - deploymentJournal = new DeploymentJournal(fileSystem, new SystemSemaphoreManager(), variables, new SilentLog()); + deploymentJournal = new DeploymentJournal(fileSystem, new MutexBasedNamedLockManager(), variables, new SilentLog()); packagesDirectory = Path.Combine(Path.GetTempPath(), "CalamariTestPackages"); fileSystem.EnsureDirectoryExists(packagesDirectory); diff --git a/source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/NamedLockFixtureBase.cs b/source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/NamedLockFixtureBase.cs new file mode 100644 index 0000000000..02cc6a0aec --- /dev/null +++ b/source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/NamedLockFixtureBase.cs @@ -0,0 +1,180 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Calamari.Common.Features.Processes.NamedLocks; +using NUnit.Framework; + +namespace Calamari.Tests.Fixtures.Integration.Process.NamedLocks +{ + public abstract class NamedLockFixtureBase + { + // Must comfortably exceed the 3s initial wait inside MutexBasedNamedLockManager. + static readonly TimeSpan RecoveryAllowance = TimeSpan.FromSeconds(15); + + // Held so the abandoned mutex handle is never closed or finalised during the test + Mutex abandonedMutex; + + [TearDown] + public void DropAbandonedMutex() + { + abandonedMutex = null; + } + + [Test] + public void SecondNamedLockWaitsUntilFirstIsReleased() + { + SecondWaitsUntilFirstIsReleased(new MutexBasedNamedLockManager()); + } + + [Test] + public void NamedLockShouldIsolate() + { + ShouldIsolate(new MutexBasedNamedLockManager()); + } + + // Acquire() must recover when the holder dies without running the Releaser (killed mid-ApplyRetention, + // Tentacle restart, OOM). + [Test] + public void AcquireRecoversWhenTheHolderIsAbandoned() + { + var name = $"Octopus.Calamari.AbandonedHolder.{Guid.NewGuid():N}"; + var globalName = $@"Global\{name}"; + var sut = new MutexBasedNamedLockManager(); + + // Simulate the lock being abandoned by some other process entirely + var holder = new Thread(() => + { + abandonedMutex = new Mutex(false, globalName); + abandonedMutex.WaitOne(); + }); + holder.Start(); + holder.Join(); + + var acquire = Task.Run(() => + { + using (sut.Acquire(name, "Another process is using the package journal")) + { + } + }); + + Assert.That(acquire.Wait(RecoveryAllowance), Is.True, "Acquire() never returned after the holder was abandoned."); + } + + [Test] + public void ReleasingFromADifferentThreadThanAcquiredSucceeds() + { + var name = $"Octopus.Calamari.CrossThreadRelease.{Guid.NewGuid():N}"; + var sut = new MutexBasedNamedLockManager(); + + var releaser = sut.Acquire(name, "Another process is using the package journal"); + + Exception releaseFailure = null; + var releasingThread = new Thread(() => + { + try + { + releaser.Dispose(); + } + catch (Exception ex) + { + releaseFailure = ex; + } + }); + releasingThread.Start(); + + Assert.That(releasingThread.Join(TimeSpan.FromSeconds(5)), Is.True, "Dispose() did not complete on the releasing thread."); + Assert.That(releaseFailure, Is.Null, "Disposing the releaser from a different thread threw."); + + var reacquire = Task.Run(() => + { + using (sut.Acquire(name, "Another process is using the package journal")) + { + } + }); + + Assert.That(reacquire.Wait(TimeSpan.FromSeconds(5)), Is.True, "Dispose() returned without actually releasing the mutex."); + } + + [Test] + public void DisposingTheReleaserTwiceIsANoOp() + { + var name = $"Octopus.Calamari.DoubleDispose.{Guid.NewGuid():N}"; + var sut = new MutexBasedNamedLockManager(); + + var releaser = sut.Acquire(name, "Another process is using the package journal"); + releaser.Dispose(); + + Assert.DoesNotThrow(() => releaser.Dispose(), "Disposing the releaser a second time threw."); + + var reacquire = Task.Run(() => + { + using (sut.Acquire(name, "Another process is using the package journal")) + { + } + }); + + Assert.That(reacquire.Wait(TimeSpan.FromSeconds(5)), Is.True, "The second Dispose() interfered with the released mutex."); + } + + static void ShouldIsolate(INamedLockManager namedLockManager) + { + var result = 0; + var threads = new List(); + + for (var i = 0; i < 4; i++) + { + threads.Add(new Thread(new ThreadStart(delegate + { + using (namedLockManager.Acquire("CalamariTest", "Another process is performing arithmetic, please wait")) + { + result = 1; + Thread.Sleep(200); + result = result + 1; + Thread.Sleep(200); + result = result + 1; + } + }))); + } + + foreach (var thread in threads) + thread.Start(); + + foreach (var thread in threads) + thread.Join(); + + Assert.That(result, Is.EqualTo(3)); + } + + static void SecondWaitsUntilFirstIsReleased(INamedLockManager namedLockManager) + { + AutoResetEvent autoEvent = new AutoResetEvent(false); + var threadTwoShouldGetTheLock = true; + + var threadOne = new Thread(() => + { + using (namedLockManager.Acquire("Octopus.Calamari.TestNamedLock", "Another process has the lock...")) + { + threadTwoShouldGetTheLock = false; + autoEvent.Set(); + Thread.Sleep(200); + threadTwoShouldGetTheLock = true; + } + }); + + var threadTwo = new Thread(() => + { + autoEvent.WaitOne(); + using (namedLockManager.Acquire("Octopus.Calamari.TestNamedLock", "Another process has the lock...")) + { + Assert.That(threadTwoShouldGetTheLock, Is.True); + } + }); + + threadOne.Start(); + threadTwo.Start(); + threadOne.Join(); + threadTwo.Join(); + } + } +} diff --git a/source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/NixSystemSemaphoreFixture.cs b/source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/NixNamedLockFixture.cs similarity index 52% rename from source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/NixSystemSemaphoreFixture.cs rename to source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/NixNamedLockFixture.cs index cf91569434..6c8e9cbb7d 100644 --- a/source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/NixSystemSemaphoreFixture.cs +++ b/source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/NixNamedLockFixture.cs @@ -1,11 +1,11 @@ using Calamari.Testing.Helpers; using NUnit.Framework; -namespace Calamari.Tests.Fixtures.Integration.Process.Semaphores +namespace Calamari.Tests.Fixtures.Integration.Process.NamedLocks { [TestFixture] [Category(TestCategory.CompatibleOS.OnlyNixOrMac)] - public class NixSystemSemaphoreFixture : SemaphoreFixtureBase + public class NixNamedLockFixture : NamedLockFixtureBase { } } \ No newline at end of file diff --git a/source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/WindowsSystemSemaphoreFixture.cs b/source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/WindowsNamedLockFixture.cs similarity index 50% rename from source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/WindowsSystemSemaphoreFixture.cs rename to source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/WindowsNamedLockFixture.cs index b2baacefa2..d88a373991 100644 --- a/source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/WindowsSystemSemaphoreFixture.cs +++ b/source/Calamari.Tests/Fixtures/Integration/Process/NamedLocks/WindowsNamedLockFixture.cs @@ -1,10 +1,10 @@ using Calamari.Testing.Helpers; using NUnit.Framework; -namespace Calamari.Tests.Fixtures.Integration.Process.Semaphores +namespace Calamari.Tests.Fixtures.Integration.Process.NamedLocks { [TestFixture] [Category(TestCategory.CompatibleOS.OnlyWindows)] - public class WindowsSystemSemaphoreFixture : SemaphoreFixtureBase + public class WindowsNamedLockFixture : NamedLockFixtureBase { } -} \ No newline at end of file +} diff --git a/source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/SemaphoreFixtureBase.cs b/source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/SemaphoreFixtureBase.cs deleted file mode 100644 index 3ebf146ec1..0000000000 --- a/source/Calamari.Tests/Fixtures/Integration/Process/Semaphores/SemaphoreFixtureBase.cs +++ /dev/null @@ -1,83 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Threading; -using Calamari.Common.Features.Processes.Semaphores; -using NUnit.Framework; - -namespace Calamari.Tests.Fixtures.Integration.Process.Semaphores -{ - public abstract class SemaphoreFixtureBase - { - [Test] - public void SystemSemaphoreWaitsUntilFirstSemaphoreIsReleased() - { - SecondSemaphoreWaitsUntilFirstSemaphoreIsReleased(new SystemSemaphoreManager()); - } - - [Test] - public void SystemSemaphoreShouldIsolate() - { - ShouldIsolate(new SystemSemaphoreManager()); - } - - static void ShouldIsolate(ISemaphoreFactory semaphore) - { - var result = 0; - var threads = new List(); - - for (var i = 0; i < 4; i++) - { - threads.Add(new Thread(new ThreadStart(delegate - { - using (semaphore.Acquire("CalamariTest", "Another process is performing arithmetic, please wait")) - { - result = 1; - Thread.Sleep(200); - result = result + 1; - Thread.Sleep(200); - result = result + 1; - } - }))); - } - - foreach (var thread in threads) - thread.Start(); - - foreach (var thread in threads) - thread.Join(); - - Assert.That(result, Is.EqualTo(3)); - } - - static void SecondSemaphoreWaitsUntilFirstSemaphoreIsReleased(ISemaphoreFactory semaphore) - { - AutoResetEvent autoEvent = new AutoResetEvent(false); - var threadTwoShouldGetSemaphore = true; - - var threadOne = new Thread(() => - { - using (semaphore.Acquire("Octopus.Calamari.TestSemaphore", "Another process has the semaphore...")) - { - threadTwoShouldGetSemaphore = false; - autoEvent.Set(); - Thread.Sleep(200); - threadTwoShouldGetSemaphore = true; - } - }); - - var threadTwo = new Thread(() => - { - autoEvent.WaitOne(); - using (semaphore.Acquire("Octopus.Calamari.TestSemaphore", "Another process has the semaphore...")) - { - Assert.That(threadTwoShouldGetSemaphore, Is.True); - } - }); - - threadOne.Start(); - threadTwo.Start(); - threadOne.Join(); - threadTwo.Join(); - } - } -} diff --git a/source/Calamari.Tests/Fixtures/PackageRetention/JournalFixture.cs b/source/Calamari.Tests/Fixtures/PackageRetention/JournalFixture.cs index 4b06e6c491..5a4181512b 100644 --- a/source/Calamari.Tests/Fixtures/PackageRetention/JournalFixture.cs +++ b/source/Calamari.Tests/Fixtures/PackageRetention/JournalFixture.cs @@ -2,7 +2,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.Deployment.PackageRetention; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Logging; @@ -40,7 +40,7 @@ public void Setup() Substitute.For(), Substitute.For(), Substitute.For>(), - Substitute.For() + Substitute.For() ); } @@ -169,7 +169,7 @@ public void WhenRetentionIsApplied_ThenPackageFileAndUsageAreRemoved() Substitute.For(), fileSystem, new []{ retentionAlgorithm }, - Substitute.For()); + Substitute.For()); thisJournal.RegisterPackageUse(packageOne, new ServerTaskId("Deployment-1"), 1000); thisJournal.ApplyRetention(); @@ -200,7 +200,7 @@ public void WhenRetentionIsAppliedAndCacheSpaceIsNotSufficient_ThenPackageFileAn Substitute.For(), fileSystem, new []{ retentionAlgorithm }, - Substitute.For()); + Substitute.For()); thisJournal.RegisterPackageUse(existingPackage, new ServerTaskId("Deployment-1"), 1 * 1024 * 1024); //Package is 1 MB thisJournal.ApplyRetention(); @@ -231,7 +231,7 @@ public void WhenRetentionIsAppliedAndCacheSpaceIsSufficientButDiskSpaceIsNot_The Substitute.For(), fileSystem, new []{ retentionAlgorithm }, - Substitute.For()); + Substitute.For()); thisJournal.RegisterPackageUse(existingPackage, new ServerTaskId("Deployment-1"), 1 * 1024 * 1024); //Package is 1 MB thisJournal.ApplyRetention(); @@ -265,7 +265,7 @@ public void WhenStaleLocksAreExpired_TheLocksAreRemoved() Substitute.For(), Substitute.For(), Substitute.For>(), - Substitute.For()); + Substitute.For()); testJournal.ExpireStaleLocks(TimeSpan.FromDays(14)); Assert.IsFalse(testJournalRepository.HasLock(thePackage)); @@ -301,7 +301,7 @@ public void OnlyStaleLocksAreExpired() Substitute.For(), Substitute.For(), Substitute.For>(), - Substitute.For()); + Substitute.For()); testJournal.ExpireStaleLocks(TimeSpan.FromDays(14)); Assert.IsFalse(testJournalRepository.HasLock(packageOne)); diff --git a/source/Calamari.Tests/Fixtures/PackageRetention/LeastFrequentlyUsedWithAgingSortPerformanceFixture.cs b/source/Calamari.Tests/Fixtures/PackageRetention/LeastFrequentlyUsedWithAgingSortPerformanceFixture.cs index cba128a0b1..0dbc0bad97 100644 --- a/source/Calamari.Tests/Fixtures/PackageRetention/LeastFrequentlyUsedWithAgingSortPerformanceFixture.cs +++ b/source/Calamari.Tests/Fixtures/PackageRetention/LeastFrequentlyUsedWithAgingSortPerformanceFixture.cs @@ -2,7 +2,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.Deployment.PackageRetention; using Calamari.Common.Plumbing.Logging; using Calamari.Deployment.PackageRetention.Caching; @@ -42,7 +42,7 @@ static InMemoryJournalRepository SeedJournal() Substitute.For(), new TestCalamariPhysicalFileSystem(), Substitute.For>(), - new SystemSemaphoreManager()); + new MutexBasedNamedLockManager()); var serverTask = new ServerTaskId("ServerTasks-1"); for (var i = 0; i < 50000; i++) { diff --git a/source/Calamari/Commands/CleanCommand.cs b/source/Calamari/Commands/CleanCommand.cs index 049d962cc2..e9203e8329 100644 --- a/source/Calamari/Commands/CleanCommand.cs +++ b/source/Calamari/Commands/CleanCommand.cs @@ -1,7 +1,7 @@ using Calamari.Commands.Support; using Calamari.Common.Commands; using Calamari.Common.Features.Deployment.Journal; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Logging; @@ -42,7 +42,7 @@ public override int Execute(string[] commandLineArguments) if (days <=0 && releases <= 0) throw new CommandException("A value must be provided for either --days or --releases"); - var deploymentJournal = new DeploymentJournal(fileSystem, new SystemSemaphoreManager(), variables, log); + var deploymentJournal = new DeploymentJournal(fileSystem, new MutexBasedNamedLockManager(), variables, log); var clock = new SystemClock(); var retentionPolicy = new RetentionPolicy(fileSystem, deploymentJournal, clock, log); diff --git a/source/Calamari/Commands/DeployPackageCommand.cs b/source/Calamari/Commands/DeployPackageCommand.cs index bc64582213..5be0ce41d5 100644 --- a/source/Calamari/Commands/DeployPackageCommand.cs +++ b/source/Calamari/Commands/DeployPackageCommand.cs @@ -11,7 +11,7 @@ using Calamari.Common.Features.EmbeddedResources; using Calamari.Common.Features.Packages; using Calamari.Common.Features.Processes; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Features.Scripting; using Calamari.Common.Features.StructuredVariables; using Calamari.Common.Features.Substitutions; @@ -101,8 +101,8 @@ public override int Execute(string[] commandLineArguments) featureClasses.Add(new NginxFeature(NginxServer.AutoDetect(), fileSystem, log)); } - var semaphore = new SystemSemaphoreManager(); - var journal = new DeploymentJournal(fileSystem, semaphore, variables, log); + var namedLockManager = new MutexBasedNamedLockManager(); + var journal = new DeploymentJournal(fileSystem, namedLockManager, variables, log); var conventions = new List { diff --git a/source/Calamari/Commands/Java/DeployJavaArchiveCommand.cs b/source/Calamari/Commands/Java/DeployJavaArchiveCommand.cs index ff035c3c03..a33992d5cf 100644 --- a/source/Calamari/Commands/Java/DeployJavaArchiveCommand.cs +++ b/source/Calamari/Commands/Java/DeployJavaArchiveCommand.cs @@ -11,7 +11,7 @@ using Calamari.Common.Features.Packages.Decorators; using Calamari.Common.Features.Packages.Java; using Calamari.Common.Features.Processes; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Features.Scripting; using Calamari.Common.Features.StructuredVariables; using Calamari.Common.Features.Substitutions; @@ -78,8 +78,8 @@ public override int Execute(string[] commandLineArguments) log.Info("Deploying: " + archiveFile); - var semaphore = new SystemSemaphoreManager(); - var journal = new DeploymentJournal(fileSystem, semaphore, variables, log); + var namedLockManager = new MutexBasedNamedLockManager(); + var journal = new DeploymentJournal(fileSystem, namedLockManager, variables, log); var jarTools = new JarTool(commandLineRunner, log, fileSystem, variables); var packageExtractor = new JarPackageExtractor(jarTools).WithExtractionLimits(log, variables); var embeddedResources = new AssemblyEmbeddedResources(); diff --git a/source/Calamari/Commands/TransferPackageCommand.cs b/source/Calamari/Commands/TransferPackageCommand.cs index 745a04555c..d0cc4075c5 100644 --- a/source/Calamari/Commands/TransferPackageCommand.cs +++ b/source/Calamari/Commands/TransferPackageCommand.cs @@ -3,7 +3,7 @@ using Calamari.Commands.Support; using Calamari.Common.Commands; using Calamari.Common.Features.Deployment.Journal; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.Deployment.Journal; using Calamari.Common.Plumbing.Extensions; using Calamari.Common.Plumbing.FileSystem; @@ -37,7 +37,7 @@ public override int Execute(string[] commandLineArguments) if (packageFile == null) // required: true in the above call means it will throw rather than return null, but there's no way to tell the compiler that. And ! doesn't work in older frameworks throw new CommandException("Package File path could not be determined"); - var journal = new DeploymentJournal(fileSystem, new SystemSemaphoreManager(), variables, log); + var journal = new DeploymentJournal(fileSystem, new MutexBasedNamedLockManager(), variables, log); var conventions = new List { diff --git a/source/Calamari/Deployment/PackageRetention/Model/PackageJournal.cs b/source/Calamari/Deployment/PackageRetention/Model/PackageJournal.cs index 4025c03672..91cbcb3720 100644 --- a/source/Calamari/Deployment/PackageRetention/Model/PackageJournal.cs +++ b/source/Calamari/Deployment/PackageRetention/Model/PackageJournal.cs @@ -1,7 +1,7 @@ using System; using System.Collections.Generic; using System.Linq; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.Deployment.PackageRetention; using Calamari.Common.Plumbing.FileSystem; using Calamari.Common.Plumbing.Logging; @@ -16,26 +16,26 @@ public class PackageJournal : IManagePackageCache readonly IRetentionAlgorithm[] retentionAlgorithms; readonly ILog log; readonly ICalamariFileSystem fileSystem; - readonly ISemaphoreFactory semaphoreFactory; + readonly INamedLockManager namedLockManager; public PackageJournal(IJournalRepository journalRepository, ILog log, ICalamariFileSystem fileSystem, IEnumerable retentionAlgorithms, - ISemaphoreFactory semaphoreFactory) + INamedLockManager namedLockManager) { this.journalRepository = journalRepository; this.log = log; this.fileSystem = fileSystem; this.retentionAlgorithms = retentionAlgorithms.ToArray(); - this.semaphoreFactory = semaphoreFactory; + this.namedLockManager = namedLockManager; } public void RegisterPackageUse(PackageIdentity package, ServerTaskId deploymentTaskId, ulong packageSizeBytes) { try { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { journalRepository.Load(); journalRepository.Cache.IncrementCacheAge(); @@ -66,7 +66,7 @@ public void RegisterPackageUse(PackageIdentity package, ServerTaskId deploymentT public void RemoveAllLocks(ServerTaskId serverTaskId) { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { log.Verbose($"Releasing package locks for task {serverTaskId}"); journalRepository.Load(); @@ -79,7 +79,7 @@ public void ApplyRetention() { try { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { journalRepository.Load(); var packagesToRemove = retentionAlgorithms.SelectMany(algorithm => algorithm.GetPackagesToRemove(journalRepository.GetAllJournalEntries())); @@ -110,7 +110,7 @@ public void ExpireStaleLocks(TimeSpan timeBeforeExpiration) { try { - using (AcquireSemaphore()) + using (AcquireNamedLock()) { journalRepository.Load(); foreach (var entry in journalRepository.GetAllJournalEntries()) @@ -134,9 +134,9 @@ public void ExpireStaleLocks(TimeSpan timeBeforeExpiration) } } - IDisposable AcquireSemaphore() + IDisposable AcquireNamedLock() { - return semaphoreFactory.Acquire(nameof(PackageJournal), "Another process is using the package journal"); + return namedLockManager.Acquire(nameof(PackageJournal), "Another process is using the package journal"); } } } \ No newline at end of file diff --git a/source/Calamari/Program.cs b/source/Calamari/Program.cs index 91751d4ad5..5185c38d1d 100644 --- a/source/Calamari/Program.cs +++ b/source/Calamari/Program.cs @@ -13,7 +13,7 @@ using Calamari.Common; using Calamari.Common.Commands; using Calamari.Common.Features.Discovery; -using Calamari.Common.Features.Processes.Semaphores; +using Calamari.Common.Features.Processes.NamedLocks; using Calamari.Common.Plumbing.Commands; using Calamari.Common.Plumbing.Deployment.Journal; using Calamari.Common.Plumbing.Deployment.PackageRetention; @@ -119,7 +119,7 @@ protected override void ConfigureContainer(ContainerBuilder builder, CommonOptio .As() .SingleInstance(); - builder.RegisterInstance(new SystemSemaphoreManager()).As(); + builder.RegisterInstance(new MutexBasedNamedLockManager()).As(); TypeDescriptor.AddAttributes(typeof(ServerTaskId), new TypeConverterAttribute(typeof(TinyTypeTypeConverter)));