SendInBackground should not use ThreadStatic

[snakefoot]

https://github.com/MindscapeHQ/raygun4net/blob/0f232cd9829bcda2d769ae7a26249a149a041d12/Mindscape.Raygun4Net4/RaygunClient.cs#L335

Should avoid populating ThreadStatic variables for ThreadPool-Threads.

Passing parameters using global variables is generally a bad idea.

[knumat]

This is a problem, since the thread that performs the async operation may not be the same thread that queued the operation. If not, the variables will be null, and the error message will be sent without the expected context.

Note that some versions of the library use ThreadLocal instead of ThreadStatic, but that doesn't fix this potential issue. For example: https://github.com/MindscapeHQ/raygun4net/blob/fefc40dc0e5d40691b968c6fa69ab60184457293/Mindscape.Raygun4Net.AspNetCore/RaygunClient.cs#L17

For async-based operations, you should use AsyncLocal instead of ThreadLocal, as shown in the following link. I don't know what the equivalent is in the older style of async programming that uses ThreadPool.QueueUserWorkItem(). https://docs.microsoft.com/en-us/dotnet/api/system.threading.asynclocal-1?view=netcore-3.1

Let me know if a pull request would be helpful.

[mduncan26]

Hi @knumat

Thank you for the additional information. We definitely are open to pull requests. Please feel free to open pull request for any additional you feel you can provide solutions to. We really do appreciate the community helping resolve these outstanding problems. I will also raise this issue with the team and we will look to create an internal ticket to address this issue. We will update this thread if there is any change to the progress of addressing this issue.

Thank you, Mitchell.

[knumat]

I created a PR with a couple items, but I think there is still an over-arching issue to resolve. While the ASP.NET Core version might be fixed by the PR, the remaining versions still have a potential issue. If one thread starts an operation, and a different thread continues the operation, the ThreadStatic/ThreadLocal variables will be null, leading to possible data loss. I'm not familiar enough with each platform's scheduler to know how much of an issue this might be.

[Obi-Dann]

Hello, is there any update on it? There's still an open PR awaiting for a review. And it's still an issue that ThreadLocal is used instead of AsyncLocal

[phillip-haydon]

@Obi-Dann thank you for bumping this, @xenolightning and I are looking into this, we have a big PR on the way to fix a bunch of stuff #506 so once we finish this PR we will get this issue resolved.

[phillip-haydon]

This should now be resolved in V9.0.2 for .NET Framework (#517) and V10.0.0-pre-1 should resolve it for NET Core (#518)