bugsnag-dotnet icon indicating copy to clipboard operation
bugsnag-dotnet copied to clipboard

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

Open oldcookie opened this issue 2 years ago • 2 comments

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.

oldcookie avatar Jun 30 '23 20:06 oldcookie

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 avatar Jul 04 '23 08:07 johnkiely1

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

oldcookie avatar Jul 05 '23 14:07 oldcookie