Work around 64 bit RyuJIT ThreadAbortException bug on the .NET Framework#137
Conversation
|
I have a fix for the style issue in the CI build ready. Let me know if you'd prefer a force push or another commit... |
|
@jdasilva-olo: another commit is fine, thanks. |
ogaca-dd
left a comment
ogaca-dd
left a comment
There was a problem hiding this comment.
Thanks a lot for this PR!
It looks good to me, just having a single question.
| } | ||
| } | ||
| } | ||
| #if !NETSTANDARD1_3 |
There was a problem hiding this comment.
Can you give more information on why using !NETSTANDARD1_3? Should it be NETFRAMEWORK?
There was a problem hiding this comment.
Yeah, good catch, I don't think there was a reason. NETFRAMEWORK will be better for adding/changing targets, which I did use on the test. 🤷♂️
There was a problem hiding this comment.
Ah, so I remembered today that I was thinking since ThreadAbortException exists in .NET Standard 2.0, and a .NET Standard DLL could run under the .NET Framework this would allow the fix to work in that scenario, while the extra code would be meaningless when it ran under .NET Core. That said, since you include assemblies targeting the .NET Framework in the NuGet, I'm not sure you can get to that scenario under normal circumstances.
There was a problem hiding this comment.
Thanks for the detailed explanation!
3 participants
This fixes #136
I had to add
net461as a target framework to the tests to be able to get a failing test. A few additional minor changes to the test code were needed to make it compile.The new test only has the potential to fail in 64-bit release mode using RyuJIT. With ReleaseMode selected Resharper uses 64-bit by default, but for Visual Studio you can select 64-bit in the
Process Architecture for AnyCPU projectsmenu option.ThreadAbortException exists but is not used in .NET Core so this change will have no impact.
If you comment out the fix and since it's release mode add a Console.WriteLine here, you should be able to see the constant output of repeating ThreadAbortExceptions.