Add the DisposeInvoker util class

Closes #1730

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
 {
 	/// <inheritdoc />
-	sealed class ChatTrackingContext : IChatTrackingContext
+	sealed class ChatTrackingContext : DisposeInvoker, IChatTrackingContext
 	{
 		/// <inheritdoc />
 		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<ChatTrackingContext> logger;
 
 		/// <summary>
-		/// <see langword="lock"/> <see cref="object"/> for modifying <see cref="onDispose"/>, <see cref="channelSink"/>, and <see cref="Channels"/>.
+		/// <see langword="lock"/> <see cref="object"/> for modifying <see cref="Channels"/> and calling <see cref="IChannelSink.UpdateChannels(IEnumerable{ChannelRepresentation}, CancellationToken)"/>.
 		/// </summary>
 		readonly object synchronizationLock;
 
+		/// <summary>
+		/// The <see cref="IChannelSink"/> if any.
+		/// </summary>
+		volatile IChannelSink? channelSink;
+
 		/// <summary>
 		/// Backing field for <see cref="CustomCommands"/>.
 		/// </summary>
 		IReadOnlyCollection<CustomCommand> customCommands;
 
-		/// <summary>
-		/// The <see cref="IChannelSink"/> if any.
-		/// </summary>
-		IChannelSink? channelSink;
-
-		/// <summary>
-		/// The <see cref="Action"/> to run when <see cref="Dispose"/>d.
-		/// </summary>
-		Action? onDispose;
-
 		/// <summary>
 		/// Backing field for <see cref="Active"/>.
 		/// </summary>
@@ -91,45 +87,31 @@ namespace Tgstation.Server.Host.Components.Chat
 		/// <param name="customCommandHandler">The value of <see cref="customCommandHandler"/>.</param>
 		/// <param name="initialChannels">The initial value of <see cref="Channels"/>.</param>
 		/// <param name="logger">The value of <see cref="logger"/>.</param>
-		/// <param name="onDispose">The value of <see cref="onDispose"/>.</param>
+		/// <param name="disposeAction">The <see cref="IDisposable.Dispose"/> action for the <see cref="DisposeInvoker"/>.</param>
 		public ChatTrackingContext(
 			ICustomCommandHandler customCommandHandler,
 			IEnumerable<ChannelRepresentation> initialChannels,
 			ILogger<ChatTrackingContext> 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<CustomCommand>();
 		}
 
-		/// <inheritdoc />
-		public void Dispose()
-		{
-			lock (synchronizationLock)
-			{
-				onDispose?.Invoke();
-				onDispose = null;
-			}
-		}
-
 		/// <inheritdoc />
 		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!");
 		}
 
 		/// <inheritdoc />

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)

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
 		/// <summary>
 		/// The <see cref="Action"/> to run when <see cref="DisposeAsync"/> is called.
 		/// </summary>
-		Action? onDispose;
+		DisposeInvoker? onDispose;
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="DmbProvider"/> class.
@@ -42,7 +43,7 @@ namespace Tgstation.Server.Host.Components.Deployment
 		/// <param name="ioManager">The value of <see cref="ioManager"/>.</param>
 		/// <param name="onDispose">The value of <see cref="onDispose"/>.</param>
 		/// <param name="directoryAppend">The optional value of <see cref="directoryAppend"/>.</param>
-		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
 		/// <inheritdoc />
 		public override ValueTask DisposeAsync()
 		{
-			onDispose?.Invoke();
+			onDispose?.Dispose();
 			return ValueTask.CompletedTask;
 		}
 

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
 {
 	/// <inheritdoc />
-	sealed class BridgeRegistration : IBridgeRegistration
+	sealed class BridgeRegistration : DisposeInvoker, IBridgeRegistration
 	{
-		/// <summary>
-		/// <see langword="lock"/> <see cref="object"/> for accessing <see cref="onDispose"/>.
-		/// </summary>
-		readonly object lockObject;
-
-		/// <summary>
-		/// <see cref="Action"/> to run when <see cref="Dispose"/>d.
-		/// </summary>
-		Action onDispose;
-
 		/// <summary>
 		/// Initializes a new instance of the <see cref="BridgeRegistration"/> class.
 		/// </summary>
-		/// <param name="onDispose">The value of <see cref="onDispose"/>.</param>
-		public BridgeRegistration(Action onDispose)
+		/// <param name="disposeAction">The <see cref="IDisposable.Dispose"/> action for the <see cref="DisposeInvoker"/>.</param>
+		public BridgeRegistration(Action disposeAction)
+			: base(disposeAction)
 		{
-			this.onDispose = onDispose ?? throw new ArgumentNullException(nameof(onDispose));
-			lockObject = new object();
-		}
-
-		/// <inheritdoc />
-		public void Dispose()
-		{
-			lock (lockObject)
-			{
-				onDispose?.Invoke();
-				onDispose = null;
-			}
 		}
 	}
 }

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
 {
 	/// <inheritdoc />
 #pragma warning disable CA1506 // TODO: Decomplexify
-	sealed class Repository : IRepository
+	sealed class Repository : DisposeInvoker, IRepository
 	{
 		/// <summary>
 		/// The default username for committers.
@@ -117,16 +118,6 @@ namespace Tgstation.Server.Host.Components.Repository
 		/// </summary>
 		readonly GeneralConfiguration generalConfiguration;
 
-		/// <summary>
-		/// <see cref="Action"/> to be taken when <see cref="Dispose"/> is called.
-		/// </summary>
-		readonly Action onDispose;
-
-		/// <summary>
-		/// If the <see cref="Repository"/> was disposed.
-		/// </summary>
-		bool disposed;
-
 		/// <summary>
 		/// Initializes a new instance of the <see cref="Repository"/> class.
 		/// </summary>
@@ -139,7 +130,7 @@ namespace Tgstation.Server.Host.Components.Repository
 		/// <param name="gitRemoteFeaturesFactory">The <see cref="IGitRemoteFeaturesFactory"/> to provide the value of <see cref="gitRemoteFeatures"/>.</param>
 		/// <param name="logger">The value of <see cref="logger"/>.</param>
 		/// <param name="generalConfiguration">The value of <see cref="generalConfiguration"/>.</param>
-		/// <param name="onDispose">The value if <see cref="onDispose"/>.</param>
+		/// <param name="disposeAction">The <see cref="IDisposable.Dispose"/> action for the <see cref="DisposeInvoker"/>.</param>
 		public Repository(
 			LibGit2Sharp.IRepository libGitRepo,
 			ILibGit2Commands commands,
@@ -150,7 +141,8 @@ namespace Tgstation.Server.Host.Components.Repository
 			IGitRemoteFeaturesFactory gitRemoteFeaturesFactory,
 			ILogger<Repository> 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);
 		}
 
-		/// <inheritdoc />
-		public void Dispose()
-		{
-			lock (onDispose)
-			{
-				if (disposed)
-					return;
-
-				disposed = true;
-			}
-
-			logger.LogTrace("Disposing...");
-			libGitRepo.Dispose();
-			onDispose();
-		}
-
 		/// <inheritdoc />
 #pragma warning disable CA1506 // TODO: Decomplexify
 		public async ValueTask<TestMergeResult> AddTestMerge(
@@ -869,6 +844,14 @@ namespace Tgstation.Server.Host.Components.Repository
 			DefaultIOManager.BlockingTaskCreationOptions,
 			TaskScheduler.Current);
 
+		/// <inheritdoc />
+		protected override void DisposeImpl()
+		{
+			logger.LogTrace("Disposing...");
+			libGitRepo.Dispose();
+			base.DisposeImpl();
+		}
+
 		/// <summary>
 		/// Runs a blocking force checkout to <paramref name="committish"/>.
 		/// </summary>

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
 	{
 		/// <summary>
-		/// The <see cref="Dispose"/> <see cref="Action"/>.
+		/// The <see cref="DisposeInvoker"/>.
 		/// </summary>
-		readonly Action? onDispose;
+		readonly DisposeInvoker? disposeInvoker;
 
 		/// <summary>
 		/// Initializes a new instance of the <see cref="RestartRegistration"/> class.
 		/// </summary>
-		/// <param name="onDispose">The value of <see cref="onDispose"/>.</param>
-		public RestartRegistration(Action? onDispose)
+		/// <param name="disposeInvoker">The value of <see cref="disposeInvoker"/>.</param>
+		public RestartRegistration(DisposeInvoker? disposeInvoker)
 		{
-			this.onDispose = onDispose;
+			this.disposeInvoker = disposeInvoker;
 		}
 
 		/// <inheritdoc />
-		public void Dispose() => onDispose?.Invoke();
+		public void Dispose() => disposeInvoker?.Dispose();
 	}
 }

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);

src/Tgstation.Server.Host/Utils/DisposeInvoker.cs

@@ -0,0 +1,49 @@
+using System;
+using System.Threading;
+
+namespace Tgstation.Server.Host.Utils
+{
+	/// <summary>
+	/// Runs a given <see cref="disposeAction"/> on <see cref="Dispose"/>.
+	/// </summary>
+	class DisposeInvoker : IDisposable
+	{
+		/// <summary>
+		/// If <see cref="Dispose"/> was called.
+		/// </summary>
+		public bool IsDisposed => disposeRan != 0;
+
+		/// <summary>
+		/// The <see cref="Action"/> to run on <see cref="Dispose"/>.
+		/// </summary>
+		readonly Action disposeAction;
+
+		/// <summary>
+		/// An <see cref="int"/> representation of a <see cref="bool"/> indicating if <see cref="Dispose"/> has ran.
+		/// </summary>
+		volatile int disposeRan;
+
+		/// <summary>
+		/// Initializes a new instance of the <see cref="DisposeInvoker"/> class.
+		/// </summary>
+		/// <param name="disposeAction">The value of <see cref="disposeAction"/>.</param>
+		public DisposeInvoker(Action disposeAction)
+		{
+			this.disposeAction = disposeAction ?? throw new ArgumentNullException(nameof(disposeAction));
+		}
+
+		/// <inheritdoc />
+		public void Dispose()
+		{
+			if (Interlocked.Exchange(ref disposeRan, 1) != 0)
+				return;
+
+			DisposeImpl();
+		}
+
+		/// <summary>
+		/// Implementation of <see cref="Dispose"/> run after reentrancy check.
+		/// </summary>
+		protected virtual void DisposeImpl() => disposeAction();
+	}
+}