Skip to content

Commit 9bd2e23

Browse files
thomhurstclaude
andcommitted
fix: Address review feedback for caching improvements
- 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 <noreply@anthropic.com>
1 parent 37d9df2 commit 9bd2e23

2 files changed

Lines changed: 19 additions & 28 deletions

File tree

src/ModularPipelines.Azure/AzureKeyVault.cs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,25 +16,27 @@ namespace ModularPipelines.Azure;
1616
/// </remarks>
1717
internal class AzureKeyVault : IAzureKeyVault
1818
{
19-
private readonly ConcurrentDictionary<string, SecretClient> _secretClients = new();
20-
private readonly ConcurrentDictionary<string, CertificateClient> _certificateClients = new();
21-
private readonly ConcurrentDictionary<string, KeyClient> _keyClients = new();
19+
// Use composite key (vaultUri, credential) to ensure different credentials for the same vault
20+
// return different clients. TokenCredential uses reference equality.
21+
private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), SecretClient> _secretClients = new();
22+
private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), CertificateClient> _certificateClients = new();
23+
private readonly ConcurrentDictionary<(string VaultUri, TokenCredential Credential), KeyClient> _keyClients = new();
2224

2325
public SecretClient GetSecretClient(Uri vaultUri, TokenCredential tokenCredential)
2426
{
25-
var key = vaultUri.ToString();
27+
var key = (vaultUri.ToString(), tokenCredential);
2628
return _secretClients.GetOrAdd(key, _ => new SecretClient(vaultUri, tokenCredential));
2729
}
2830

2931
public CertificateClient GetCertificateClient(Uri vaultUri, TokenCredential tokenCredential)
3032
{
31-
var key = vaultUri.ToString();
33+
var key = (vaultUri.ToString(), tokenCredential);
3234
return _certificateClients.GetOrAdd(key, _ => new CertificateClient(vaultUri, tokenCredential));
3335
}
3436

3537
public KeyClient GetKeyClient(Uri vaultUri, TokenCredential tokenCredential)
3638
{
37-
var key = vaultUri.ToString();
39+
var key = (vaultUri.ToString(), tokenCredential);
3840
return _keyClients.GetOrAdd(key, _ => new KeyClient(vaultUri, tokenCredential));
3941
}
4042
}

src/ModularPipelines/Context/Hasher.cs

Lines changed: 11 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -7,23 +7,14 @@ namespace ModularPipelines.Context;
77
/// Provides hashing operations using various algorithms.
88
/// </summary>
99
/// <remarks>
10-
/// HashAlgorithm instances are not thread-safe, so we use ThreadLocal to cache
11-
/// one instance per thread. This avoids the allocation overhead of creating new
12-
/// instances on every call while maintaining thread safety.
10+
/// Uses the static HashData methods available in .NET 5+ which are thread-safe
11+
/// and don't require disposal, avoiding resource leaks.
1312
/// </remarks>
1413
internal class Hasher : IHasher
1514
{
1615
private readonly IHex _hex;
1716
private readonly IBase64 _base64;
1817

19-
// ThreadLocal instances for thread-safe caching of hash algorithms
20-
// HashAlgorithm is not thread-safe, so each thread gets its own instance
21-
private static readonly ThreadLocal<SHA1> Sha1Algorithm = new(() => SHA1.Create());
22-
private static readonly ThreadLocal<SHA256> Sha256Algorithm = new(() => SHA256.Create());
23-
private static readonly ThreadLocal<SHA384> Sha384Algorithm = new(() => SHA384.Create());
24-
private static readonly ThreadLocal<SHA512> Sha512Algorithm = new(() => SHA512.Create());
25-
private static readonly ThreadLocal<MD5> Md5Algorithm = new(() => MD5.Create());
26-
2718
public Hasher(IHex hex, IBase64 base64)
2819
{
2920
_hex = hex;
@@ -32,33 +23,31 @@ public Hasher(IHex hex, IBase64 base64)
3223

3324
public string Sha1(string input, HashType hashType = HashType.Hex)
3425
{
35-
return ComputeHash(Sha1Algorithm.Value!, input, hashType);
26+
var bytes = System.Security.Cryptography.SHA1.HashData(Encoding.UTF8.GetBytes(input));
27+
return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes);
3628
}
3729

3830
public string Sha256(string input, HashType hashType = HashType.Hex)
3931
{
40-
return ComputeHash(Sha256Algorithm.Value!, input, hashType);
32+
var bytes = System.Security.Cryptography.SHA256.HashData(Encoding.UTF8.GetBytes(input));
33+
return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes);
4134
}
4235

4336
public string Sha384(string input, HashType hashType = HashType.Hex)
4437
{
45-
return ComputeHash(Sha384Algorithm.Value!, input, hashType);
38+
var bytes = System.Security.Cryptography.SHA384.HashData(Encoding.UTF8.GetBytes(input));
39+
return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes);
4640
}
4741

4842
public string Sha512(string input, HashType hashType = HashType.Hex)
4943
{
50-
return ComputeHash(Sha512Algorithm.Value!, input, hashType);
44+
var bytes = System.Security.Cryptography.SHA512.HashData(Encoding.UTF8.GetBytes(input));
45+
return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes);
5146
}
5247

5348
public string Md5(string input, HashType hashType = HashType.Hex)
5449
{
55-
return ComputeHash(Md5Algorithm.Value!, input, hashType);
56-
}
57-
58-
private string ComputeHash(HashAlgorithm hashAlgorithm, string input, HashType hashType)
59-
{
60-
var bytes = hashAlgorithm.ComputeHash(Encoding.UTF8.GetBytes(input));
61-
50+
var bytes = System.Security.Cryptography.MD5.HashData(Encoding.UTF8.GetBytes(input));
6251
return hashType == HashType.Hex ? _hex.ToHex(bytes) : _base64.ToBase64String(bytes);
6352
}
6453
}

0 commit comments

Comments
 (0)