-
Notifications
You must be signed in to change notification settings - Fork 549
[debugger] Work around a debugger issue when using the interpreter #15451
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
Conversation
| }; | ||
|
|
||
| if (xamarin_init_mono_debug) | ||
| mono_main (2, (char**)argc); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain runtime-wise why this works around the problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, sure.
The issue on runtime side was that we are initializing interpreter before setting the debugger attributes. So when we execute this code: https://github.com/dotnet/runtime/blob/5529dc79e9f35d70e9d01318d31844984b860dd4/src/mono/mono/mini/interp/interp.c#L8121
mdb_optimizations was false, then we keep the default optimizations of the interpreter which was breaking the debugger experience.
In this workaround I'm passing --interp=-all in the parameters, so I'm disabling the interpreter optimizations and the debugger works well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this only a problem in .NET 6? Or legacy mono as well? (i.e. mono/mono's 2020-02 branch)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm almost sure that it's a problem only on .net 6, but I would ask someone to test to double check. It's really easy to reproduce:
public static void MethodMyMethod()
{
var e2 = new Exception("hi thays");
var e1 = e2;
if (e1.Equals(e2))
Console.WriteLine("equals");
}
Adding a breakpoint on if statement and looking at locals, with the optimizations turned on we only see the content of variable e2, e1 is null.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this going to have implications for hot reload on .NET MAUI?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so. @lambdageek do you have any thoughts about this workaround?
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
the xtra failures are interesting, is complaining due to a binding problem, when it should not happen. Merge with main, if issues are the same I'll fix the PR or main |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
runtime/monovm-bridge.m
Outdated
| #if DOTNET | ||
| bool use_mono_workaround = xamarin_init_mono_debug; | ||
| #else | ||
| bool use_mono_workaround = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rolfbjarne I also had a repro with Hot Restart in Xamarin Forms and after discussing with @thaystg, she confirmed that this bug might also repro in a mono runtime environment so I think we should consider the workaround for non dotnet scenarios
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to follow up: legacy scenarios will be fixed with #15507.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Mac Catalyst apps crash with like this with this fix: https://gist.github.com/rolfbjarne/293ecffb7fcf3e4169494b81d7e4cd97 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This is exactly the scenario that I was testing and I don't see this crash: Then I debug the app and I can see the locals variables correctly. |
You need this commit for it to crash reliably: Otherwise it only crashes on macOS 10 and macOS 11, on newer versions you get random behavior (which happens to work). |
💻 [PR Build] Tests on macOS Mac Catalina (10.15) passed 💻✅ All tests on macOS Mac Catalina (10.15) passed. Pipeline on Agent |
✅ API diff for current PR / commitLegacy Xamarin (No breaking changes)
NET (empty diffs)
✅ API diff vs stableLegacy Xamarin (No breaking changes).NET (No breaking changes)✅ Generator diffGenerator diff is empty Pipeline on Agent |
❌ [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) failed ❌Failed tests are:
Pipeline on Agent |
🔥 [CI Build] Test results 🔥Test results❌ Tests failed on VSTS: simulator tests 0 tests crashed, 1 tests failed, 222 tests passed. Failures❌ dotnettests testsDetails
Html Report (VSDrops) Download Successes✅ bcl: All 69 tests passed. Html Report (VSDrops) Download Pipeline on Agent |
|
Test failures are unrelated (https://github.com/xamarin/maccore/issues/2595). |
…otnet#15451) There's a bug in the Mono runtime where the interpreter does not disable optimizations when the debugger is attached, which leads to the interpreter optimizing code and the debugger ending up rather confused. The bug is fixed in the Mono runtime (dotnet/runtime#71436), but there's no immediate way for the runtime to release this fix, so here we're implementing a workaround that disables interpreter optimizations if the debugging is enabled. It's somewhat clunky because the Mono external API wasn't designed for this, so we have to abuse the API a bit to accomplish the effect we want. This is somewhat risky (since we're changing the startup path in a pretty big way), but there's an escape hatch via an environment variable, and also the workaround will not be in effect for release builds. While the runtime issue exists in legacy mono/mono as well, we'll fix the Mono runtime for legacy, because we don't have to wait to consume legacy mono (dotnet#15507). This means that the workaround is for .NET scenarios only. Co-authored-by: Manuel de la Pena <[email protected]> Co-authored-by: Rolf Bjarne Kvinge <[email protected]>
There's a bug in the Mono runtime where the interpreter does not disable optimizations when the debugger is attached, which leads to the interpreter optimizing code and the debugger ending up rather confused. The bug is fixed in the Mono runtime (dotnet/runtime#71436), but there's no immediate way for the runtime to release this fix, so here we're implementing a workaround that disables interpreter optimizations if the debugging is enabled. It's somewhat clunky because the Mono external API wasn't designed for this, so we have to abuse the API a bit to accomplish the effect we want. This is somewhat risky (since we're changing the startup path in a pretty big way), but there's an escape hatch via an environment variable, and also the workaround will not be in effect for release builds. While the runtime issue exists in legacy mono/mono as well, we'll fix the Mono runtime for legacy, because we don't have to wait to consume legacy mono (dotnet#15507). This means that the workaround is for .NET scenarios only. This is a backport of #dotnet#15451.
There's a bug in the Mono runtime where the interpreter does not disable optimizations when the debugger is attached, which leads to the interpreter optimizing code and the debugger ending up rather confused. The bug is fixed in the Mono runtime (dotnet/runtime#71436), but there's no immediate way for the runtime to release this fix, so here we're implementing a workaround that disables interpreter optimizations if the debugging is enabled. It's somewhat clunky because the Mono external API wasn't designed for this, so we have to abuse the API a bit to accomplish the effect we want. This is somewhat risky (since we're changing the startup path in a pretty big way), but there's an escape hatch via an environment variable, and also the workaround will not be in effect for release builds. While the runtime issue exists in legacy mono/mono as well, we'll fix the Mono runtime for legacy, because we don't have to wait to consume legacy mono (dotnet#15507). This means that the workaround is for .NET scenarios only. This is a backport of #dotnet#15451.
…15510) There's a bug in the Mono runtime where the interpreter does not disable optimizations when the debugger is attached, which leads to the interpreter optimizing code and the debugger ending up rather confused. The bug is fixed in the Mono runtime (dotnet/runtime#71436), but there's no immediate way for the runtime to release this fix, so here we're implementing a workaround that disables interpreter optimizations if the debugging is enabled. It's somewhat clunky because the Mono external API wasn't designed for this, so we have to abuse the API a bit to accomplish the effect we want. This is somewhat risky (since we're changing the startup path in a pretty big way), but there's an escape hatch via an environment variable, and also the workaround will not be in effect for release builds. While the runtime issue exists in legacy mono/mono as well, we'll fix the Mono runtime for legacy, because we don't have to wait to consume legacy mono (#15507). This means that the workaround is for .NET scenarios only. This is a backport of ##15451. Co-authored-by: Thays Grazia <[email protected]>
…15509) There's a bug in the Mono runtime where the interpreter does not disable optimizations when the debugger is attached, which leads to the interpreter optimizing code and the debugger ending up rather confused. The bug is fixed in the Mono runtime (dotnet/runtime#71436), but there's no immediate way for the runtime to release this fix, so here we're implementing a workaround that disables interpreter optimizations if the debugging is enabled. It's somewhat clunky because the Mono external API wasn't designed for this, so we have to abuse the API a bit to accomplish the effect we want. This is somewhat risky (since we're changing the startup path in a pretty big way), but there's an escape hatch via an environment variable, and also the workaround will not be in effect for release builds. While the runtime issue exists in legacy mono/mono as well, we'll fix the Mono runtime for legacy, because we don't have to wait to consume legacy mono (#15507). This means that the workaround is for .NET scenarios only. This is a backport of ##15451. Co-authored-by: Thays Grazia <[email protected]>
TODO:
Only start runtime using mono_main when it's on interpreter mode and debug enabled.
CONS:
With this implementation we see in the log a message like this:
Maybe we could pass on args --version then we would see the mono version but wouldn't see this help message.