DuendeSoftware / Support

Support for Duende Software products
21 stars 0 forks source link

Hitting .well-known/openid-configuration endpoint leads to memory leak #1397

Open bhosale-ajay opened 2 months ago

bhosale-ajay commented 2 months ago

Which version of Duende IdentityServer are you using? 7.0.6

Which version of .NET are you using? 8.0

Describe the bug If hit /.well-known/openid-configuration multiple times it leads to memory leak. Server is configured as follows

.Services
        .AddIdentityServer(options => {
              // set license keys, login, and logout urls etc
        })
       .AddInMemoryCaching()
       .AddConfigurationStore(options =>
        {
              //
        })
        .AddConfigurationStoreCache()
        .AddOperationalStore(options =>
        {
               //
        })
        .AddInMemoryIdentityResources(IdentityConfig.IdentityResources)
        .AddInMemoryApiScopes(IdentityConfig.Scopes)
        .AddInMemoryApiResources(IdentityConfig.Resources)
        .AddInMemoryClients(IdentityConfig.GetClients(fullBaseUrl, snowBaseUrl, snowSecretValue))
        .AddProfileService<CustomProfileService>();

To Reproduce

Steps to reproduce the behavior.

Expected behavior No memory leak should be observed.

Kindly let me know if there is any to way cache the output, and why adding AddInMemoryCaching is not sufficient.

bhosale-ajay commented 2 months ago

Considering everything else is same, if there are two endpoints within same asp .net project

  1. /.well-known/openid-configuration

  2. /ping which just returns a "pong" as response

If 10,000 non-concurrent requests made then mem usage looks like below

image

AndersAbel commented 2 months ago

Thanks for sharing these details. We have another issue that we are working on that gives timeouts on the discovery endpoint. I wounder if this could actually be the same issue. The memory leakage could be an early indication that something is wrong and then it would eventually freeze.

Could you please check your dependencies according to the notes in https://github.com/DuendeSoftware/Support/issues/1361#issuecomment-2352570891?

bhosale-ajay commented 2 months ago

We do not have dependency on "Azure.Extensions.AspNetCore.DataProtection.Keys", But have dependency on "Azure.Identity" (1.12.0) and "Azure.Core" (1.41.0). Do note that our services run on Docker image mcr.microsoft.com/dotnet/aspnet:8.0.3-alpine3.19-amd64

bhosale-ajay commented 2 months ago

Our configurations are not dynamic. Is it possible to cache the response? The current code prepares /.well-known response by adds lots of entries to a Dictionary<string, object> and then serializing it to Json.

AndersAbel commented 2 months ago

We do not have dependency on "Azure.Extensions.AspNetCore.DataProtection.Keys", But have dependency on "Azure.Identity" (1.12.0) and "Azure.Core" (1.41.0). Do note that our services run on Docker image mcr.microsoft.com/dotnet/aspnet:8.0.3-alpine3.19-amd64

The issue is in Azure.Core version 1.41.0 and prior, could you please try updating Azure.Core to 1.42.0? There is o new Azure.Identity package that forces the dependency to be updated, so you would have to add a direct reference to Azure.Core version 1.42.0 to your project.

bhosale-ajay commented 2 months ago

Updated to Azure Core 1.43, and no changes to memory consumption. My worry is that memory is not getting released.

image

damianh commented 5 days ago

Hi @bhosale-ajay , a quick update, we are attempting to reproduce this internally. Could you kindly share your DataProtection setup? Are you using KeyVault / BlobStorage? Thanks.

bhosale-ajay commented 5 days ago

Do you mean this? Not using KeyVault or Blob Storage. Using postgres.

    builder.Services.AddDataProtection()
        .UseCryptographicAlgorithms(new AuthenticatedEncryptorConfiguration
        {
            EncryptionAlgorithm = EncryptionAlgorithm.AES_256_CBC,
            ValidationAlgorithm = ValidationAlgorithm.HMACSHA256,
        })
        .PersistKeysToDbContext<SecurityContext>()
        .SetApplicationName(ApplicationName);
damianh commented 1 day ago

Our configurations are not dynamic. Is it possible to cache the response? The current code prepares /.well-known response by adds lots of entries to a Dictionary<string, object> and then serializing it to Json.

I think we can do some optimisation here alright but not sure there's a memory leak.

Been doing some tests and I see memory growth when hammering .well-known/openid-configuration however it tops out at ~1GB (an interesting round number) and GC kicks in:

Image

This is with <ServerGarbageCollection>true</ServerGarbageCollection>. My understanding is that with this setting, memory usage will increase depending on amount available to the system. The process continues like this with no indications of further memory growth/leakage.

To compare, I ran the same test with <ServerGarbageCollection>false</ServerGarbageCollection> (aka "workstation" GC):

Image

Here the memory is topping out at 68MB and maintaining it at that level. Obviously the GC is doing more work here managing the heap but it looks fairly level.

It's looking like to me that the memory leak is just normal memory growth experienced with ServerGarbageCollection set to true.

Thoughts?