-
Notifications
You must be signed in to change notification settings - Fork 52
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.
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
AddJwtBearerAuthentication method waits indefinitely #126
Comments
Hi @chetan-vangala, Thanks for reporting this issue and for providing such helpful details. As you mentioned, that piece of code can be the cause of the deadlock you are experiencing. Since Also, I have a question for you. Did the MS team provide you with any other suggestion/recommendation that we should take into account? |
Hi @laura-rodriguez, Thanks for the quick response! Once they helped us narrow down the issue to this point, their only real recommendation was reach out to your team first since you have the extra context of having built the library and might have a workaround or resolution that we/they weren't aware of. |
Hi @laura-rodriguez , Have there been any updates here? Or in lieu of that, can you let me know what sort of timeframe I can expect to hear back in? |
Hi @chetan-vangala, This task is on our backlog already, and we are planning to start working on this during the current sprint. We reached out to the project Katana team, and we are going to implement the workaround that was suggested here. Stay tuned for updates. |
Hi @chetan-vangala, We have a PR that should fix this issue. Would you mind checking out this fix in your project to verify that it actually fixes the problem? That would require you to check out the branch and reference the |
Hey @laura-rodriguez, This is great news, thank you! I think we should be able to test it out like this - unfortunately it'll take a bit to test since it's an issue that takes a few days to occur. We can kind of only test it by running it and waiting for a few days to confirm that the issue doesn't happen. I'll report back after we've had a chance to verify! |
Hi @chetan-vangala, We appreciate your help with this! ❤️ Can you provide us an ETA of when you think we would hear back from you? Thank you! |
Hi @laura-rodriguez, Likewise! :) I asked the team to deploy this change to our test environment today. If they were able to do so and we don't see the issue reoccur by Tuesday I think we'll feel fairly confident in the issue being resolved with this fix. I'll check in with them again Monday morning. Worst case they should be able to deploy on Monday - in which case we'd probably want to give it until Thursday/Friday. |
Hi @laura-rodriguez, Sorry for the delay here - our team has tried to swap out the nuget package we were using with manual references for the dlls build from your project, but for some reason we're having trouble with the dependencies. Any chance I can take you up on the test nuget package offer from earlier? |
Hi @chetan-vangala , Sure! You can find the test NuGet packages attached here in a zip file. Let me know how it goes :). |
Hi @chetan-vangala , Were you guys able to try the test NuGet packages? |
Hi @laura-rodriguez , There was some delay with the deployment, but they got them up to our test environment. Now we wait! if the deadlock hasn't happened by Monday/Tuesday it's likely that the fix worked. |
Hi @chetan-vangala, Do you have any updates on this? Thanks! |
Hi @laura-rodriguez, Sorry for the delayed response here - there were some issues with deployment that were unrelated to this fix that we needed to resolve. The fix has been deployed for 3 days now though without the issue reoccurring. Normally the issue would have happened within this timeframe, so I feel pretty confident that your fix has worked. We're still waiting for a little longer to close this issue on our end, but I don't foresee any change in status. Thanks for all of your help with this! When this is merged into a release and available through nuget online, will that information be available here? When that happens we'll be the first to update our packages. :) |
Hi @chetan-vangala , Thanks for the update! I'm glad to know that the issue hasn't showed up. We are currently working on preparing a new release, so this will be available soon. Thanks for your help in getting this issue fixed ❤️ . |
Fixed in #129 . Available in |
We're using this package to authorize .NET Framework API endpoints that are eventually hosted in an Azure app service. After a certain period of time (~approx 1 day) the API stops responding to requests until the whole app service is restarted. We ended up troubleshooting this with Microsoft, which led us to capturing a memory dump (below). In that memory dump the call stack leads to a method in this library (AddJwtBearerAuthentication), which waits indefinitely when this issue is occurring.
Microsoft has suggested that in the following class https://github.com/okta/okta-aspnet/blob/master/Okta.AspNet/OktaMiddlewareExtensions.cs, on line 69 the code is handling an async method synchronously in a way that could lead to a deadlock.
IssuerSigningKeyResolver = (token, securityToken, keyId, validationParameters) => { var signingKeys = signingKeyProvider.GetSigningKeysAsync().GetAwaiter().GetResult(); return signingKeys.Where(x => x.KeyId == keyId); }
They suggested we reach out to you to see if your team is aware of this issue and/or if you have any workarounds. Please let me know if there's any additional information I can provide to help us get to a resolution. This issue has been impacting our production environment for a considerable amount of time now, and we're definitely invested in resolving it as soon as possible.
Memory dump:
Thread Stacks in the Process
Below are all the call-stacks of the threads that were captured at the end of the profiler trace (up to a maximum of 1000 threads)
5 thread(s) matching
The text was updated successfully, but these errors were encountered: