Skip to content
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

Asp.Net Client can leak memory and trigger large number of exceptions #164

Open
oldcookie opened this issue Jun 30, 2023 · 2 comments
Open
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug

Comments

@oldcookie
Copy link

Describe the bug

Asp.Net client would keep adding the same middleware to the static _globalClient if Bugsnag.Client.Current is called outside of a request scope and call to BeginNotify() is called subsequently to add a middleware.

This results in a memory leak since _globalClient is static. When Notify() is called, this also results a very long running loop and a lot of handled exceptions due to the large number of middleware linked to the globalclient.

Steps to reproduce

  1. Invoke a async function with ConfigureAwait(false) or use Task.Run()
  2. Get a BugSnag client using Bugsnag.Client.Current when the HttpContext.current is no long in scope.
  3. Add a middleware.
  4. Repeat this across many requests
  5. Trigger Notify() on the global client

You would see a lot of exceptions in performance counters.

Environment

  • Bugsnag version: N/A
  • .NET framework version: 4.7.2

BeforeNotify() should probably check if it is using the _globalClient and not add a middleware or middleware should be stored using a HashSet instead to prevent duplicates.

@johnkiely1
Copy link
Member

Hi @oldcookie, Thanks for raising we are going to look to get this fixed. In the meantime it would be interesting to know if you've managed to work around it?

@johnkiely1 johnkiely1 added bug Confirmed bug backlog We hope to fix this feature/bug in the future labels Jul 4, 2023
@oldcookie
Copy link
Author

Yes, we added a check for HttpContext.current != null before adding a middleware, which bypasses the situation where this turns into an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog We hope to fix this feature/bug in the future bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

2 participants