From 80b50a442651fa328e44147cbbb25817d15ef092 Mon Sep 17 00:00:00 2001 From: Jordan Dominion Date: Sat, 20 Apr 2024 10:34:36 -0400 Subject: [PATCH] Make `IdentityCacheObject` `IAsyncDisposable` Closes #1733 --- .../Controllers/ApiRootController.cs | 2 +- .../Security/IIdentityCache.cs | 4 +- .../Security/IdentityCache.cs | 53 +++++++++++-------- .../Security/IdentityCacheObject.cs | 16 +++--- 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/src/Tgstation.Server.Host/Controllers/ApiRootController.cs b/src/Tgstation.Server.Host/Controllers/ApiRootController.cs index fa6b3e0c4b..9bf56a0e4d 100644 --- a/src/Tgstation.Server.Host/Controllers/ApiRootController.cs +++ b/src/Tgstation.Server.Host/Controllers/ApiRootController.cs @@ -357,7 +357,7 @@ namespace Tgstation.Server.Host.Controllers var identExpiry = token.ParseJwt().ValidTo; identExpiry += tokenFactory.ValidationParameters.ClockSkew; identExpiry += TimeSpan.FromSeconds(15); - identityCache.CacheSystemIdentity(user, systemIdentity!, identExpiry); + await identityCache.CacheSystemIdentity(user, systemIdentity!, identExpiry); } Logger.LogDebug("Successfully logged in user {userId}!", user.Id); diff --git a/src/Tgstation.Server.Host/Security/IIdentityCache.cs b/src/Tgstation.Server.Host/Security/IIdentityCache.cs index 75010f2c64..4a74ed15ac 100644 --- a/src/Tgstation.Server.Host/Security/IIdentityCache.cs +++ b/src/Tgstation.Server.Host/Security/IIdentityCache.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using Tgstation.Server.Host.Models; @@ -15,7 +16,8 @@ namespace Tgstation.Server.Host.Security /// The the belongs to. /// The to cache. /// When the should expire. - void CacheSystemIdentity(User user, ISystemIdentity systemIdentity, DateTimeOffset expiry); + /// A representing the running operation. + ValueTask CacheSystemIdentity(User user, ISystemIdentity systemIdentity, DateTimeOffset expiry); /// /// Attempt to load a cached . diff --git a/src/Tgstation.Server.Host/Security/IdentityCache.cs b/src/Tgstation.Server.Host/Security/IdentityCache.cs index c42ffab3f6..1b40269101 100644 --- a/src/Tgstation.Server.Host/Security/IdentityCache.cs +++ b/src/Tgstation.Server.Host/Security/IdentityCache.cs @@ -1,16 +1,18 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; +using Tgstation.Server.Common.Extensions; using Tgstation.Server.Host.Models; using Tgstation.Server.Host.Utils; namespace Tgstation.Server.Host.Security { /// - sealed class IdentityCache : IIdentityCache, IDisposable + sealed class IdentityCache : IIdentityCache, IAsyncDisposable { /// /// The for the . @@ -41,15 +43,14 @@ namespace Tgstation.Server.Host.Security } /// - public void Dispose() + public ValueTask DisposeAsync() { logger.LogTrace("Disposing..."); - foreach (var cachedIdentity in cachedIdentities.Select(x => x.Value).ToList()) - cachedIdentity.Dispose(); + return ValueTaskExtensions.WhenAll(cachedIdentities.Select(x => x.Value.DisposeAsync())); } /// - public void CacheSystemIdentity(User user, ISystemIdentity systemIdentity, DateTimeOffset expiry) + public async ValueTask CacheSystemIdentity(User user, ISystemIdentity systemIdentity, DateTimeOffset expiry) { ArgumentNullException.ThrowIfNull(user); ArgumentNullException.ThrowIfNull(systemIdentity); @@ -57,27 +58,35 @@ namespace Tgstation.Server.Host.Security var uid = user.Require(x => x.Id); var sysId = systemIdentity.Uid; - lock (cachedIdentities) + ValueTask oldIdentityDisposal = ValueTask.CompletedTask; + try { - logger.LogDebug("Caching system identity {sysId} of user {uid}", sysId, uid); - - if (cachedIdentities.TryGetValue(uid, out var identCache)) + lock (cachedIdentities) { - logger.LogTrace("Expiring previously cached identity..."); - identCache.Dispose(); // also clears it out - } + logger.LogDebug("Caching system identity {sysId} of user {uid}", sysId, uid); - identCache = new IdentityCacheObject( - systemIdentity.Clone(), - asyncDelayer, - () => + if (cachedIdentities.TryGetValue(uid, out var identCache)) { - logger.LogDebug("Expiring system identity cache for user {uid}", uid); - lock (cachedIdentities) - cachedIdentities.Remove(uid); - }, - expiry); - cachedIdentities.Add(uid, identCache); + logger.LogTrace("Expiring previously cached identity..."); + oldIdentityDisposal = identCache.DisposeAsync(); // also clears it out + } + + identCache = new IdentityCacheObject( + systemIdentity.Clone(), + asyncDelayer, + () => + { + logger.LogDebug("Expiring system identity cache for user {uid}", uid); + lock (cachedIdentities) + cachedIdentities.Remove(uid); + }, + expiry); + cachedIdentities.Add(uid, identCache); + } + } + finally + { + await oldIdentityDisposal; } } diff --git a/src/Tgstation.Server.Host/Security/IdentityCacheObject.cs b/src/Tgstation.Server.Host/Security/IdentityCacheObject.cs index 7cc10664e5..af0e57dbd1 100644 --- a/src/Tgstation.Server.Host/Security/IdentityCacheObject.cs +++ b/src/Tgstation.Server.Host/Security/IdentityCacheObject.cs @@ -9,7 +9,7 @@ namespace Tgstation.Server.Host.Security /// /// For keeping a specific alive for a period of time. /// - sealed class IdentityCacheObject : IDisposable + sealed class IdentityCacheObject : IAsyncDisposable { /// /// The the manages. @@ -53,6 +53,9 @@ namespace Tgstation.Server.Host.Security { await asyncDelayer.Delay(expiry - now, cancellationToken); } + catch (OperationCanceledException) + { + } finally { onExpiry(); @@ -63,18 +66,11 @@ namespace Tgstation.Server.Host.Security } /// - public void Dispose() + public async ValueTask DisposeAsync() { cancellationTokenSource.Cancel(); - try - { - task.GetAwaiter().GetResult(); - } - catch (OperationCanceledException) - { - } - cancellationTokenSource.Dispose(); + await task; } } }