Memory Leak and Stability Concerns Introduced by UnhandledExceptionBridge Subscription in RaygunClientBase Constructor (#504)

[Obi-Dann]

Describe the bug

Since version 8.1.0 RaygunClientBase subscribes to UnhandledExceptionBridge.OnUnhandledException in the constructor. That means that RaygunClients will never be garbage collected, because their reference will be captured in UnhandledExceptionBridge.OnUnhandledException event handler.

In the context of AspNetCore, an instance of a raygun client is created for every request when an error occurs (link to code).

At the very best, this change introduces a memory leak. At the worse it can have a significant impact on platform stability.

Link to the PR and line introducing the change https://github.com/MindscapeHQ/raygun4net/pull/504 (this change)

Expected behavior What is the expected lifetime of RaygunClientBase? If it's supposed to be a singleton, it shouldn't be created for every request in AspNetCore. If it's supposed to be transient, it should be disposable to unsubscribe from the event handler.

[xenolightning]

You're absolutely right, and thanks for raising this.

We have been "treating" RaygunClient as a singleton more recently. But in reality this is not always the case.

In an ideal world RaygunClient should be a singleton, and we may make some adjustments to this (but this would likely be in a much later version)

We'll look to resolve this in two parts:

• The AspNetCore provider should be treating the client as a singleton
• We should be looking at WeakReference for the unhandled exceptions, so if a RaygunClient is transient, they can still be garbage collected

[Obi-Dann]

Well, if RaygunClient is considered singleton, then you might more concurrency issues. For example here. I suspect multiple exceptions can be sent in parallel using the same client, but there's no locking on _handlingRecursiveGrouping and it can lead to non-deterministic behaviour?

[xenolightning]

Well, if RaygunClient is considered singleton, then you might more concurrency issues. For example here. I suspect multiple exceptions can be sent in parallel using the same client, but there's no locking on _handlingRecursiveGrouping and it can lead to non-deterministic behaviour?

Yeah, I use considered a singleton very loosely.

We have plans to change that approach, to do this the RaygunClient needs to be fully thread-safe. Which is a fairly large departure from the current implementation.

This is likely to be a major update, with no expected date as of yet

[xenolightning]

Fixed and released in 8.2.1, which can now be found on nuget:

https://www.nuget.org/packages/Mindscape.Raygun4Net.NetCore/8.2.1