Skip to content

Remove lock when creating a SignatureProvider #2788

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 21, 2024
Merged

Remove lock when creating a SignatureProvider #2788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions build/version.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<!-- This file may be overwritten by automation. Only values allowed here are VersionPrefix and VersionSuffix. -->
<Project>
<!-- Wilson version -->
<!-- MicrosoftIdentityModelVersion -->
<PropertyGroup>
<MicrosoftIdentityModelCurrentVersion>8.0.1</MicrosoftIdentityModelCurrentVersion>

Expand All @@ -14,4 +14,4 @@
<FileVersion Condition="'$(MicrosoftIdentityModelVersion)' != '' and '$(IsCustomPreview)' != 'true' ">$(MicrosoftIdentityModelVersion).$([System.DateTime]::Now.AddYears(-2019).Year)$([System.DateTime]::Now.ToString("MMdd"))</FileVersion>
<FileVersion Condition="'$(MicrosoftIdentityModelVersion)' == ''">$(MicrosoftIdentityModelCurrentVersion).$([System.DateTime]::Now.AddYears(-2019).Year)$([System.DateTime]::Now.ToString("MMdd"))</FileVersion>
</PropertyGroup>
</Project>
</Project>
33 changes: 15 additions & 18 deletions src/Microsoft.IdentityModel.Tokens/CryptoProviderFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ public class CryptoProviderFactory
{
private static CryptoProviderFactory _default;
private static readonly ConcurrentDictionary<string, string> _typeToAlgorithmMap = new ConcurrentDictionary<string, string>();
private static readonly object _cacheLock = new object();
private static int _defaultSignatureProviderObjectPoolCacheSize = Environment.ProcessorCount * 4;
private static string _typeofAsymmetricSignatureProvider = typeof(AsymmetricSignatureProvider).ToString();
private static string _typeofSymmetricSignatureProvider = typeof(SymmetricSignatureProvider).ToString();
Expand Down Expand Up @@ -591,25 +590,23 @@ private SignatureProvider CreateSignatureProvider(SecurityKey key, string algori
return signatureProvider;
}

lock (_cacheLock)
{
if (CryptoProviderCache.TryGetSignatureProvider(key, algorithm, typeofSignatureProvider, willCreateSignatures, out signatureProvider))
{
signatureProvider.AddRef();
return signatureProvider;
}

if (!IsSupportedAlgorithm(algorithm, key))
throw LogHelper.LogExceptionMessage(new NotSupportedException(LogHelper.FormatInvariant(LogMessages.IDX10634, LogHelper.MarkAsNonPII(algorithm), key)));
if (!IsSupportedAlgorithm(algorithm, key))
throw LogHelper.LogExceptionMessage(new NotSupportedException(LogHelper.FormatInvariant(LogMessages.IDX10634, LogHelper.MarkAsNonPII(algorithm), key)));

if (createAsymmetric)
signatureProvider = new AsymmetricSignatureProvider(key, algorithm, willCreateSignatures, this);
else
signatureProvider = new SymmetricSignatureProvider(key, algorithm, willCreateSignatures);
if (createAsymmetric)
signatureProvider = new AsymmetricSignatureProvider(key, algorithm, willCreateSignatures, this);
else
signatureProvider = new SymmetricSignatureProvider(key, algorithm, willCreateSignatures);

if (ShouldCacheSignatureProvider(signatureProvider))
signatureProvider.IsCached = CryptoProviderCache.TryAdd(signatureProvider);
}
if (ShouldCacheSignatureProvider(signatureProvider))
// CryptoProviderCache.TryAdd will return false if unable to add the SignatureProvider.
// One possibility is the SignatureProvider was added between when we called TryGetSignatureProvider and here.
// SignatureProvider.IsCached will be false and CryptoProviderFactory.Release will dispose the SignatureProvider.
// Since the SignatueProvider, was never added to the cache, TryGetSignatureProvider will not return this instance, we can dispose.
// This will result in sometimes (rarely) creating a SignatureProvider that is never cached.
// The alternative is to use a lock after the call to TryGetSignatureProvider, and then check again: { TryGet, lock, TryGet }.
// This will result in excesive locking for different keys, which is common in POP scenarios.
signatureProvider.IsCached = CryptoProviderCache.TryAdd(signatureProvider);
}
else
{
Expand Down