diff --git a/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs b/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs index 1f3d196bcd..02b6675323 100644 --- a/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs +++ b/src/Tgstation.Server.Host/Components/Chat/ChatTrackingContext.cs @@ -7,16 +7,17 @@ using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Tgstation.Server.Host.Components.Chat.Commands; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Chat { /// - sealed class ChatTrackingContext : IChatTrackingContext + sealed class ChatTrackingContext : DisposeInvoker, IChatTrackingContext { /// public bool Active { - get => active && onDispose != null; + get => active && !IsDisposed; set { if (active == value) @@ -61,25 +62,20 @@ namespace Tgstation.Server.Host.Components.Chat readonly ILogger logger; /// - /// for modifying , , and . + /// for modifying and calling . /// readonly object synchronizationLock; + /// + /// The if any. + /// + volatile IChannelSink? channelSink; + /// /// Backing field for . /// IReadOnlyCollection customCommands; - /// - /// The if any. - /// - IChannelSink? channelSink; - - /// - /// The to run when d. - /// - Action? onDispose; - /// /// Backing field for . /// @@ -91,45 +87,31 @@ namespace Tgstation.Server.Host.Components.Chat /// The value of . /// The initial value of . /// The value of . - /// The value of . + /// The action for the . public ChatTrackingContext( ICustomCommandHandler customCommandHandler, IEnumerable initialChannels, ILogger logger, - Action onDispose) + Action disposeAction) + : base(disposeAction) { this.customCommandHandler = customCommandHandler ?? throw new ArgumentNullException(nameof(customCommandHandler)); Channels = initialChannels?.ToList() ?? throw new ArgumentNullException(nameof(initialChannels)); this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); - this.onDispose = onDispose ?? throw new ArgumentNullException(nameof(onDispose)); synchronizationLock = new object(); Active = true; customCommands = Array.Empty(); } - /// - public void Dispose() - { - lock (synchronizationLock) - { - onDispose?.Invoke(); - onDispose = null; - } - } - /// public void SetChannelSink(IChannelSink channelSink) { ArgumentNullException.ThrowIfNull(channelSink); - lock (synchronizationLock) - { - if (this.channelSink != null) - throw new InvalidOperationException("channelSink already set!"); - - this.channelSink = channelSink; - } + var originalValue = Interlocked.CompareExchange(ref this.channelSink, channelSink, null); + if (originalValue != null) + throw new InvalidOperationException("channelSink already set!"); } /// diff --git a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs index 0c5c43d31e..3b6695313b 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DmbFactory.cs @@ -16,6 +16,7 @@ using Tgstation.Server.Host.Components.Events; using Tgstation.Server.Host.Database; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Models; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Deployment { @@ -279,7 +280,7 @@ namespace Tgstation.Server.Host.Components.Deployment CleanRegisteredCompileJob(compileJob); } - var newProvider = new DmbProvider(compileJob, engineVersion, ioManager, CleanupAction); + var newProvider = new DmbProvider(compileJob, engineVersion, ioManager, new DisposeInvoker(CleanupAction)); try { const string LegacyADirectoryName = "A"; @@ -316,7 +317,7 @@ namespace Tgstation.Server.Host.Components.Deployment // rebuild the provider because it's using the legacy style directories // Don't dispose it logger.LogDebug("Creating legacy two folder .dmb provider targeting {aDirName} directory...", LegacyADirectoryName); - newProvider = new DmbProvider(compileJob, engineVersion, ioManager, CleanupAction, Path.DirectorySeparatorChar + LegacyADirectoryName); + newProvider = new DmbProvider(compileJob, engineVersion, ioManager, new DisposeInvoker(CleanupAction), Path.DirectorySeparatorChar + LegacyADirectoryName); } lock (jobLockCounts) diff --git a/src/Tgstation.Server.Host/Components/Deployment/DmbProvider.cs b/src/Tgstation.Server.Host/Components/Deployment/DmbProvider.cs index 4a21e4e8d2..be47aeb6b3 100644 --- a/src/Tgstation.Server.Host/Components/Deployment/DmbProvider.cs +++ b/src/Tgstation.Server.Host/Components/Deployment/DmbProvider.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using Tgstation.Server.Api.Models; using Tgstation.Server.Api.Models.Internal; using Tgstation.Server.Host.IO; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Components.Deployment { @@ -32,7 +33,7 @@ namespace Tgstation.Server.Host.Components.Deployment /// /// The to run when is called. /// - Action? onDispose; + DisposeInvoker? onDispose; /// /// Initializes a new instance of the class. @@ -42,7 +43,7 @@ namespace Tgstation.Server.Host.Components.Deployment /// The value of . /// The value of . /// The optional value of . - public DmbProvider(Models.CompileJob compileJob, EngineVersion engineVersion, IIOManager ioManager, Action onDispose, string? directoryAppend = null) + public DmbProvider(Models.CompileJob compileJob, EngineVersion engineVersion, IIOManager ioManager, DisposeInvoker onDispose, string? directoryAppend = null) { CompileJob = compileJob ?? throw new ArgumentNullException(nameof(compileJob)); EngineVersion = engineVersion ?? throw new ArgumentNullException(nameof(engineVersion)); @@ -54,7 +55,7 @@ namespace Tgstation.Server.Host.Components.Deployment /// public override ValueTask DisposeAsync() { - onDispose?.Invoke(); + onDispose?.Dispose(); return ValueTask.CompletedTask; } diff --git a/src/Tgstation.Server.Host/Components/Interop/Bridge/BridgeRegistration.cs b/src/Tgstation.Server.Host/Components/Interop/Bridge/BridgeRegistration.cs index e209a63aa6..38acfd827d 100644 --- a/src/Tgstation.Server.Host/Components/Interop/Bridge/BridgeRegistration.cs +++ b/src/Tgstation.Server.Host/Components/Interop/Bridge/BridgeRegistration.cs @@ -1,40 +1,21 @@ using System; +using Tgstation.Server.Host.Utils; + #nullable disable namespace Tgstation.Server.Host.Components.Interop.Bridge { /// - sealed class BridgeRegistration : IBridgeRegistration + sealed class BridgeRegistration : DisposeInvoker, IBridgeRegistration { - /// - /// for accessing . - /// - readonly object lockObject; - - /// - /// to run when d. - /// - Action onDispose; - /// /// Initializes a new instance of the class. /// - /// The value of . - public BridgeRegistration(Action onDispose) + /// The action for the . + public BridgeRegistration(Action disposeAction) + : base(disposeAction) { - this.onDispose = onDispose ?? throw new ArgumentNullException(nameof(onDispose)); - lockObject = new object(); - } - - /// - public void Dispose() - { - lock (lockObject) - { - onDispose?.Invoke(); - onDispose = null; - } } } } diff --git a/src/Tgstation.Server.Host/Components/Repository/Repository.cs b/src/Tgstation.Server.Host/Components/Repository/Repository.cs index 6ef027416e..c083dfdde6 100644 --- a/src/Tgstation.Server.Host/Components/Repository/Repository.cs +++ b/src/Tgstation.Server.Host/Components/Repository/Repository.cs @@ -17,6 +17,7 @@ using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Extensions; using Tgstation.Server.Host.IO; using Tgstation.Server.Host.Jobs; +using Tgstation.Server.Host.Utils; #nullable disable @@ -24,7 +25,7 @@ namespace Tgstation.Server.Host.Components.Repository { /// #pragma warning disable CA1506 // TODO: Decomplexify - sealed class Repository : IRepository + sealed class Repository : DisposeInvoker, IRepository { /// /// The default username for committers. @@ -117,16 +118,6 @@ namespace Tgstation.Server.Host.Components.Repository /// readonly GeneralConfiguration generalConfiguration; - /// - /// to be taken when is called. - /// - readonly Action onDispose; - - /// - /// If the was disposed. - /// - bool disposed; - /// /// Initializes a new instance of the class. /// @@ -139,7 +130,7 @@ namespace Tgstation.Server.Host.Components.Repository /// The to provide the value of . /// The value of . /// The value of . - /// The value if . + /// The action for the . public Repository( LibGit2Sharp.IRepository libGitRepo, ILibGit2Commands commands, @@ -150,7 +141,8 @@ namespace Tgstation.Server.Host.Components.Repository IGitRemoteFeaturesFactory gitRemoteFeaturesFactory, ILogger logger, GeneralConfiguration generalConfiguration, - Action onDispose) + Action disposeAction) + : base(disposeAction) { this.libGitRepo = libGitRepo ?? throw new ArgumentNullException(nameof(libGitRepo)); this.commands = commands ?? throw new ArgumentNullException(nameof(commands)); @@ -162,27 +154,10 @@ namespace Tgstation.Server.Host.Components.Repository this.logger = logger ?? throw new ArgumentNullException(nameof(logger)); this.generalConfiguration = generalConfiguration ?? throw new ArgumentNullException(nameof(generalConfiguration)); - this.onDispose = onDispose ?? throw new ArgumentNullException(nameof(onDispose)); gitRemoteFeatures = gitRemoteFeaturesFactory.CreateGitRemoteFeatures(this); } - /// - public void Dispose() - { - lock (onDispose) - { - if (disposed) - return; - - disposed = true; - } - - logger.LogTrace("Disposing..."); - libGitRepo.Dispose(); - onDispose(); - } - /// #pragma warning disable CA1506 // TODO: Decomplexify public async ValueTask AddTestMerge( @@ -869,6 +844,14 @@ namespace Tgstation.Server.Host.Components.Repository DefaultIOManager.BlockingTaskCreationOptions, TaskScheduler.Current); + /// + protected override void DisposeImpl() + { + logger.LogTrace("Disposing..."); + libGitRepo.Dispose(); + base.DisposeImpl(); + } + /// /// Runs a blocking force checkout to . /// diff --git a/src/Tgstation.Server.Host/Core/RestartRegistration.cs b/src/Tgstation.Server.Host/Core/RestartRegistration.cs index 8f4ab937d6..2cd24e307d 100644 --- a/src/Tgstation.Server.Host/Core/RestartRegistration.cs +++ b/src/Tgstation.Server.Host/Core/RestartRegistration.cs @@ -1,4 +1,4 @@ -using System; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Core { @@ -6,20 +6,20 @@ namespace Tgstation.Server.Host.Core sealed class RestartRegistration : IRestartRegistration { /// - /// The . + /// The . /// - readonly Action? onDispose; + readonly DisposeInvoker? disposeInvoker; /// /// Initializes a new instance of the class. /// - /// The value of . - public RestartRegistration(Action? onDispose) + /// The value of . + public RestartRegistration(DisposeInvoker? disposeInvoker) { - this.onDispose = onDispose; + this.disposeInvoker = disposeInvoker; } /// - public void Dispose() => onDispose?.Invoke(); + public void Dispose() => disposeInvoker?.Dispose(); } } diff --git a/src/Tgstation.Server.Host/Server.cs b/src/Tgstation.Server.Host/Server.cs index b832be6e87..751cd2d453 100644 --- a/src/Tgstation.Server.Host/Server.cs +++ b/src/Tgstation.Server.Host/Server.cs @@ -13,6 +13,7 @@ using Microsoft.Extensions.Options; using Tgstation.Server.Common.Extensions; using Tgstation.Server.Host.Configuration; using Tgstation.Server.Host.Core; +using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host { @@ -234,12 +235,13 @@ namespace Tgstation.Server.Host { logger.LogTrace("Registering restart handler {handlerImplementationName}...", handler); restartHandlers.Add(handler); - return new RestartRegistration(() => - { - lock (restartLock) - if (!shutdownInProgress) - restartHandlers.Remove(handler); - }); + return new RestartRegistration( + new DisposeInvoker(() => + { + lock (restartLock) + if (!shutdownInProgress) + restartHandlers.Remove(handler); + })); } logger.LogWarning("Restart handler {handlerImplementationName} register after a shutdown had begun!", handler); diff --git a/src/Tgstation.Server.Host/Utils/DisposeInvoker.cs b/src/Tgstation.Server.Host/Utils/DisposeInvoker.cs new file mode 100644 index 0000000000..b90d01318b --- /dev/null +++ b/src/Tgstation.Server.Host/Utils/DisposeInvoker.cs @@ -0,0 +1,49 @@ +using System; +using System.Threading; + +namespace Tgstation.Server.Host.Utils +{ + /// + /// Runs a given on . + /// + class DisposeInvoker : IDisposable + { + /// + /// If was called. + /// + public bool IsDisposed => disposeRan != 0; + + /// + /// The to run on . + /// + readonly Action disposeAction; + + /// + /// An representation of a indicating if has ran. + /// + volatile int disposeRan; + + /// + /// Initializes a new instance of the class. + /// + /// The value of . + public DisposeInvoker(Action disposeAction) + { + this.disposeAction = disposeAction ?? throw new ArgumentNullException(nameof(disposeAction)); + } + + /// + public void Dispose() + { + if (Interlocked.Exchange(ref disposeRan, 1) != 0) + return; + + DisposeImpl(); + } + + /// + /// Implementation of run after reentrancy check. + /// + protected virtual void DisposeImpl() => disposeAction(); + } +}