From 37d9df2f7c540c17db770465c337077b5014e20a Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 30 Dec 2025 16:17:59 +0000 Subject: [PATCH 1/3] perf: add caching for Hasher hash algorithms and Azure KeyVault clients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #1594 and #1596 - Use ThreadLocal for hash algorithm instances in Hasher class since HashAlgorithm is not thread-safe but creating new instances per call is wasteful - Cache Azure KeyVault clients (SecretClient, CertificateClient, KeyClient) in ConcurrentDictionary by vault URI since Azure SDK clients are thread-safe and designed for reuse 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/ModularPipelines.Azure/AzureKeyVault.cs | 22 ++++++++++++-- src/ModularPipelines/Context/Hasher.cs | 33 ++++++++++++++------- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/src/ModularPipelines.Azure/AzureKeyVault.cs b/src/ModularPipelines.Azure/AzureKeyVault.cs index fbc975c5e0..0bb8e04579 100644 --- a/src/ModularPipelines.Azure/AzureKeyVault.cs +++ b/src/ModularPipelines.Azure/AzureKeyVault.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using Azure.Core; using Azure.Security.KeyVault.Certificates; using Azure.Security.KeyVault.Keys; @@ -5,20 +6,35 @@ namespace ModularPipelines.Azure; +/// +/// Provides access to Azure Key Vault clients with caching. +/// +/// +/// Azure SDK clients (SecretClient, CertificateClient, KeyClient) are thread-safe and designed +/// to be long-lived and reused. This class caches clients by vault URI to avoid the overhead +/// of creating new clients on every call. +/// internal class AzureKeyVault : IAzureKeyVault { + private readonly ConcurrentDictionary _secretClients = new(); + private readonly ConcurrentDictionary _certificateClients = new(); + private readonly ConcurrentDictionary _keyClients = new(); + public SecretClient GetSecretClient(Uri vaultUri, TokenCredential tokenCredential) { - return new SecretClient(vaultUri, tokenCredential); + var key = vaultUri.ToString(); + return _secretClients.GetOrAdd(key, _ => new SecretClient(vaultUri, tokenCredential)); } public CertificateClient GetCertificateClient(Uri vaultUri, TokenCredential tokenCredential) { - return new CertificateClient(vaultUri, tokenCredential); + var key = vaultUri.ToString(); + return _certificateClients.GetOrAdd(key, _ => new CertificateClient(vaultUri, tokenCredential)); } public KeyClient GetKeyClient(Uri vaultUri, TokenCredential tokenCredential) { - return new KeyClient(vaultUri, tokenCredential); + var key = vaultUri.ToString(); + return _keyClients.GetOrAdd(key, _ => new KeyClient(vaultUri, tokenCredential)); } } \ No newline at end of file diff --git a/src/ModularPipelines/Context/Hasher.cs b/src/ModularPipelines/Context/Hasher.cs index e99d3f3c3a..7faddd42f1 100644 --- a/src/ModularPipelines/Context/Hasher.cs +++ b/src/ModularPipelines/Context/Hasher.cs @@ -3,11 +3,27 @@ namespace ModularPipelines.Context; +/// +/// Provides hashing operations using various algorithms. +/// +/// +/// HashAlgorithm instances are not thread-safe, so we use ThreadLocal to cache +/// one instance per thread. This avoids the allocation overhead of creating new +/// instances on every call while maintaining thread safety. +/// internal class Hasher : IHasher { private readonly IHex _hex; private readonly IBase64 _base64; + // ThreadLocal instances for thread-safe caching of hash algorithms + // HashAlgorithm is not thread-safe, so each thread gets its own instance + private static readonly ThreadLocal Sha1Algorithm = new(() => SHA1.Create()); + private static readonly ThreadLocal Sha256Algorithm = new(() => SHA256.Create()); + private static readonly ThreadLocal Sha384Algorithm = new(() => SHA384.Create()); + private static readonly ThreadLocal Sha512Algorithm = new(() => SHA512.Create()); + private static readonly ThreadLocal Md5Algorithm = new(() => MD5.Create()); + public Hasher(IHex hex, IBase64 base64) { _hex = hex; @@ -16,36 +32,33 @@ public Hasher(IHex hex, IBase64 base64) public string Sha1(string input, HashType hashType = HashType.Hex) { - return ComputeHash(SHA1.Create(), input, hashType); + return ComputeHash(Sha1Algorithm.Value!, input, hashType); } public string Sha256(string input, HashType hashType = HashType.Hex) { - return ComputeHash(SHA256.Create(), input, hashType); + return ComputeHash(Sha256Algorithm.Value!, input, hashType); } public string Sha384(string input, HashType hashType = HashType.Hex) { - return ComputeHash(SHA384.Create(), input, hashType); + return ComputeHash(Sha384Algorithm.Value!, input, hashType); } public string Sha512(string input, HashType hashType = HashType.Hex) { - return ComputeHash(SHA512.Create(), input, hashType); + return ComputeHash(Sha512Algorithm.Value!, input, hashType); } public string Md5(string input, HashType hashType = HashType.Hex) { - return ComputeHash(MD5.Create(), input, hashType); + return ComputeHash(Md5Algorithm.Value!, input, hashType); } private string ComputeHash(HashAlgorithm hashAlgorithm, string input, HashType hashType) { - using (hashAlgorithm) - { - var bytes = hashAlgorithm.ComputeHash(Encoding.UTF8.GetBytes(input)); + var bytes = hashAlgorithm.ComputeHash(Encoding.UTF8.GetBytes(input)); - return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes); - } + return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes); } } \ No newline at end of file From 9bd2e233aa672a70c908c2afdff9ef59eef63279 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 30 Dec 2025 16:46:26 +0000 Subject: [PATCH 2/3] fix: Address review feedback for caching improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Hasher: Use static HashData methods (.NET 5+) instead of ThreadLocal to avoid resource leaks from undisposed HashAlgorithm instances - AzureKeyVault: Use composite key (vaultUri, tokenCredential) to ensure different credentials for the same vault return different clients 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/ModularPipelines.Azure/AzureKeyVault.cs | 14 +++++---- src/ModularPipelines/Context/Hasher.cs | 33 +++++++-------------- 2 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/ModularPipelines.Azure/AzureKeyVault.cs b/src/ModularPipelines.Azure/AzureKeyVault.cs index 0bb8e04579..7a3d20d785 100644 --- a/src/ModularPipelines.Azure/AzureKeyVault.cs +++ b/src/ModularPipelines.Azure/AzureKeyVault.cs @@ -16,25 +16,27 @@ namespace ModularPipelines.Azure; /// internal class AzureKeyVault : IAzureKeyVault { - private readonly ConcurrentDictionary _secretClients = new(); - private readonly ConcurrentDictionary _certificateClients = new(); - private readonly ConcurrentDictionary _keyClients = new(); + // Use composite key (vaultUri, credential) to ensure different credentials for the same vault + // return different clients. TokenCredential uses reference equality. + private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), SecretClient> _secretClients = new(); + private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), CertificateClient> _certificateClients = new(); + private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), KeyClient> _keyClients = new(); public SecretClient GetSecretClient(Uri vaultUri, TokenCredential tokenCredential) { - var key = vaultUri.ToString(); + var key = (vaultUri.ToString(), tokenCredential); return _secretClients.GetOrAdd(key, _ => new SecretClient(vaultUri, tokenCredential)); } public CertificateClient GetCertificateClient(Uri vaultUri, TokenCredential tokenCredential) { - var key = vaultUri.ToString(); + var key = (vaultUri.ToString(), tokenCredential); return _certificateClients.GetOrAdd(key, _ => new CertificateClient(vaultUri, tokenCredential)); } public KeyClient GetKeyClient(Uri vaultUri, TokenCredential tokenCredential) { - var key = vaultUri.ToString(); + var key = (vaultUri.ToString(), tokenCredential); return _keyClients.GetOrAdd(key, _ => new KeyClient(vaultUri, tokenCredential)); } } \ No newline at end of file diff --git a/src/ModularPipelines/Context/Hasher.cs b/src/ModularPipelines/Context/Hasher.cs index 7faddd42f1..325b6b7244 100644 --- a/src/ModularPipelines/Context/Hasher.cs +++ b/src/ModularPipelines/Context/Hasher.cs @@ -7,23 +7,14 @@ namespace ModularPipelines.Context; /// Provides hashing operations using various algorithms. /// /// -/// HashAlgorithm instances are not thread-safe, so we use ThreadLocal to cache -/// one instance per thread. This avoids the allocation overhead of creating new -/// instances on every call while maintaining thread safety. +/// Uses the static HashData methods available in .NET 5+ which are thread-safe +/// and don't require disposal, avoiding resource leaks. /// internal class Hasher : IHasher { private readonly IHex _hex; private readonly IBase64 _base64; - // ThreadLocal instances for thread-safe caching of hash algorithms - // HashAlgorithm is not thread-safe, so each thread gets its own instance - private static readonly ThreadLocal Sha1Algorithm = new(() => SHA1.Create()); - private static readonly ThreadLocal Sha256Algorithm = new(() => SHA256.Create()); - private static readonly ThreadLocal Sha384Algorithm = new(() => SHA384.Create()); - private static readonly ThreadLocal Sha512Algorithm = new(() => SHA512.Create()); - private static readonly ThreadLocal Md5Algorithm = new(() => MD5.Create()); - public Hasher(IHex hex, IBase64 base64) { _hex = hex; @@ -32,33 +23,31 @@ public Hasher(IHex hex, IBase64 base64) public string Sha1(string input, HashType hashType = HashType.Hex) { - return ComputeHash(Sha1Algorithm.Value!, input, hashType); + var bytes = System.Security.Cryptography.SHA1.HashData(Encoding.UTF8.GetBytes(input)); + return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes); } public string Sha256(string input, HashType hashType = HashType.Hex) { - return ComputeHash(Sha256Algorithm.Value!, input, hashType); + var bytes = System.Security.Cryptography.SHA256.HashData(Encoding.UTF8.GetBytes(input)); + return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes); } public string Sha384(string input, HashType hashType = HashType.Hex) { - return ComputeHash(Sha384Algorithm.Value!, input, hashType); + var bytes = System.Security.Cryptography.SHA384.HashData(Encoding.UTF8.GetBytes(input)); + return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes); } public string Sha512(string input, HashType hashType = HashType.Hex) { - return ComputeHash(Sha512Algorithm.Value!, input, hashType); + var bytes = System.Security.Cryptography.SHA512.HashData(Encoding.UTF8.GetBytes(input)); + return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes); } public string Md5(string input, HashType hashType = HashType.Hex) { - return ComputeHash(Md5Algorithm.Value!, input, hashType); - } - - private string ComputeHash(HashAlgorithm hashAlgorithm, string input, HashType hashType) - { - var bytes = hashAlgorithm.ComputeHash(Encoding.UTF8.GetBytes(input)); - + var bytes = System.Security.Cryptography.MD5.HashData(Encoding.UTF8.GetBytes(input)); return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes); } } \ No newline at end of file From cb2153ba5ce79bec7accd968d1adeda9a76c2f10 Mon Sep 17 00:00:00 2001 From: Tom Longhurst <30480171+thomhurst@users.noreply.github.com> Date: Tue, 30 Dec 2025 16:52:48 +0000 Subject: [PATCH 3/3] fix: Key Azure clients by vault URI only with documentation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TokenCredential lacks value equality, so including it in the cache key causes cache misses for logically identical credentials. Since this service is Scoped, the same credential is typically used throughout a pipeline execution. Added documentation explaining this behavior. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- src/ModularPipelines.Azure/AzureKeyVault.cs | 25 ++++++++++++++------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/ModularPipelines.Azure/AzureKeyVault.cs b/src/ModularPipelines.Azure/AzureKeyVault.cs index 7a3d20d785..0b71ac657d 100644 --- a/src/ModularPipelines.Azure/AzureKeyVault.cs +++ b/src/ModularPipelines.Azure/AzureKeyVault.cs @@ -10,33 +10,42 @@ namespace ModularPipelines.Azure; /// Provides access to Azure Key Vault clients with caching. /// /// +/// /// Azure SDK clients (SecretClient, CertificateClient, KeyClient) are thread-safe and designed /// to be long-lived and reused. This class caches clients by vault URI to avoid the overhead /// of creating new clients on every call. +/// +/// +/// Important: Clients are cached by vault URI only. Within a scope, the first +/// TokenCredential used for a vault URI will be used for all subsequent requests to that vault. +/// This is intentional since AzureKeyVault is registered as Scoped and typically the same +/// credential is used throughout a pipeline execution scope. +/// /// internal class AzureKeyVault : IAzureKeyVault { - // Use composite key (vaultUri, credential) to ensure different credentials for the same vault - // return different clients. TokenCredential uses reference equality. - private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), SecretClient> _secretClients = new(); - private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), CertificateClient> _certificateClients = new(); - private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), KeyClient> _keyClients = new(); + // Cache by vault URI only. Within a scope, the same credential is typically used. + // TokenCredential lacks value equality, so including it in the key would cause cache misses + // for logically identical credentials (e.g., two DefaultAzureCredential instances). + private readonly ConcurrentDictionary _secretClients = new(); + private readonly ConcurrentDictionary _certificateClients = new(); + private readonly ConcurrentDictionary _keyClients = new(); public SecretClient GetSecretClient(Uri vaultUri, TokenCredential tokenCredential) { - var key = (vaultUri.ToString(), tokenCredential); + var key = vaultUri.ToString(); return _secretClients.GetOrAdd(key, _ => new SecretClient(vaultUri, tokenCredential)); } public CertificateClient GetCertificateClient(Uri vaultUri, TokenCredential tokenCredential) { - var key = (vaultUri.ToString(), tokenCredential); + var key = vaultUri.ToString(); return _certificateClients.GetOrAdd(key, _ => new CertificateClient(vaultUri, tokenCredential)); } public KeyClient GetKeyClient(Uri vaultUri, TokenCredential tokenCredential) { - var key = (vaultUri.ToString(), tokenCredential); + var key = vaultUri.ToString(); return _keyClients.GetOrAdd(key, _ => new KeyClient(vaultUri, tokenCredential)); } } \ No newline at end of file