-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[Hot Reload] metadata.c:1326, condition `GINT_TO_UINT32(idx) < table_info_get_rows (t)' not met #93860
Comments
From @rolfbjarne on Fri, 20 Oct 2023 08:44:49 GMT @jeromelaban this looks like a crash in the debugger; does the app run if you don't attach a debugger (just tap on it)? |
From @jeromelaban on Fri, 20 Oct 2023 11:30:28 GMT @rolfbjarne thanks for looking into it! No, the app does not crash, but there's no hot reload that can happen either. The issue only happens when explicitly triggering a hot reload (which happens through the debugger in this case). It may be a runtime issue, though, since it fails in a very similar way under Android. I can open a runtime issue if you think it's more appropriate. |
From @jeromelaban on Fri, 20 Oct 2023 12:36:24 GMT @rolfbjarne I just discovered that I missed some steps in the original description, my apologies. Hot reloading is required for this to fail. |
From @rolfbjarne on Mon, 23 Oct 2023 12:15:07 GMT
Yes, this looks like a runtime issue, so the dotnet/runtime repository would be a better place for it. I'll move it there. |
@rolfbjarne I was under the impression that using |
Also, the issue happens on webassembly, which makes sense as it's mono underneath. (/cc @lewing) |
CreateNewOnMetadataUpdate is hard to use (the old class name is still around and valid) and I don't think Maui has been updated to use it (although I could be wrong). But the runtime crash is not expected. |
@jeromelaban do you have separate repro steps for Wasm? I think it's probably crashing in a different place for the same underlying reason. It would be helpful to see the other stack trace |
@lambdageek I'll try to build one using blazor. In the meantime it really is almost the same place, probably in slight version differences:
|
I'm still not able to get a repro with blazor, but here's another log, which includes the call to Here's an excerpt of the log:
Update: Still trying to get a repro, and it seems to be related to nested types marked as |
@jeromelaban unfortunately this part ofhte trace:
is not useful. that's just the part where the runtime is reporting that there was a failed assertion. what I really need is the symbolic names for this part:
The line in On ios it is clearly something in the debugger. But on wasm I wouldn't expect the updates to come in via the debugger (and I don't think the debugger is active right - you're just running @lewing @pavelsavara do we have some way so symbolicate wasm crashes? |
Tagging subscribers to 'arch-wasm': @lewing Issue DetailsFrom @jeromelaban on Fri, 20 Oct 2023 03:53:31 GMT Steps to Reproduce
Expected BehaviorThe code is updated without crashing. Actual BehaviorSometimes:
Sometimes:
Environment
Build LogsAvailable on demand. Copied from original issue xamarin/xamarin-macios#19277
|
This is not the case, btw. The debugger and the managed API call down into identical code. The only way in which not using the debugger is more stable is that there is less code observing the hot reload changes. (The debugger does things like query the names of locals, get all the members of a class, etc etc, that typical code could do with reflection, but is pretty rare in general). |
Thanks for the updates @lambdageek.
Agreed it's not useful at this point, I'm trying to get a repro for blazor specifically and have symbols for that trace as well, no luck so far (having both the crash and symbols enabled).
For wasm it's not using the debugger, indeed, the browserlink feature is used to provide the updates. Aside from that, on the iOS front from my own Uno tests, I enable traces for everything, and here's what I got:
That being said, the crash on MAUI is somewhat different (using the repro steps at the top): 93860-crash_trace_ios_maui.txt, so maybe it's two separate issues, after all. |
@lambdageek I've been trying to get to a repro for the To repro on macOS:
Notice that the app will crash with the metadata.c:1326 assertion. The app has all runtime debugging enabled for easier troubleshooting. |
@jeromelaban I can repro the Uno crash. It's always on the second update for me. crash reporter shows this stack:
which looks like there was already some fault and then the I'm going to try to make some custom builds of the runtime for iossimulator with extra logging. by the way, is it possible to build the project for iossimulator-arm64. My naive attempts failed (the build tried to link some SkiaSharp native library that was built for ios not iossimulator and at that point I gave up). Not a big deal (I can make iossimulator-x64 custom runtimes, so I'm not blocked), just curious. |
@lambdageek it is supposed to be supported, that's what SkiaSharp 2.88.6 was about, but I'll try on arm64 and let you know if there's something missing (could be the native assets for iOS that are incorrect). We could also completely remove all skiasharp references, they're not needed for this issue, but I'll post an updated sample without it so you don't have to fiddle. |
@lambdageek I was not able to fail the build with the original sample on arm64, but here's an updated one without Skia dependencies: issue83860-2.zip. |
@lambdageek Furthermore, seeing the stacktrace that you posted got me thinking that the interpreter could play a role, but it's not, even when disabling it. Adding: <MtouchExtraArgs>$(MtouchExtraArgs) --setenv=DOTNET_MODIFIABLE_ASSEMBLIES=debug</MtouchExtraArgs> (because there's no interpreter) does not help and the failure in metadata.c is the same. |
yea that would be useful. although i'm not sure if wasm has the same problems or different problems :D I'm still trying to understand why in the Uno iossimulator repro something is trying to throw an exception after a hot reload change is applied. The table_info_get_rows crash is masking some other problem - and i'm trying to figure out what that is. |
@jeromelaban with the new zip file, when I following your reproduction steps, I ran into the following error.
It seems that |
@fanyang-mono thanks for trying out the repro! This is curious that you're not getting the app to build. In the terminal that builds the app, are there any errors that are showing up? Otherwise, there's a binlog that is generated in the folder of the mobile project which may contain more information about what's not working. I'm also wondering, it seems that you're building for x64, are you on an x64 mac as well? |
@jeromelaban Yes, I am using mac x64 |
@fanyang-mono thanks, then the build error may be specific to your local setup for iOS and iOS workloads, somehow. The binlog I mentioned is very likely to provide additional information. |
@lambdageek I just finished validating the fix from #94540 on Wasm, it works! Thanks for working on it! |
@jeromelaban I'm getting some weird behavior when I try to use my local build of I end up with a crash like this:
I added some extra printfs and I can tell that I somehow ended up with two copies of Also I still don't understand what is forcing my VS Code to build for iossimulator-x64 rather than iossimulator-arm64. Not sure if it's related. (When I hit ctrl-F5, the build terminal says: |
Turning off the static registrar didn't help. So that's not it. |
@lambdageek I'm not sure why x64 is selected, I'm looking into it. But you should be able to build and run directly using |
Also, you should still be able to test hotreload, if the folder was opened in VS Code. |
I think the "overwrite libmonosgen-2.0.dylib" trick doesn't quite work for ios. having trouble with it with a plain (arm64) maui app, too. so I'm going to backport the assertion fix to net8 since it is clearly beneficial (our new unit test shows it helps with new code that throws) but I can't guarantee it will actually fix all the problems for Uno's hot reload on ios. @rolfbjarne is there some way to validate a runtime change with a released version of the ios SDK? everything I'm trying results in suspicious crashes that look like multiple copies of the runtime in the final app. |
@lambdageek I would probably:
I believe @ivanpovazan had a different (and possibly easier) way to do this though. |
@lambdageek one thing that might help is adding the following target in the project file: <Target Name="UpdateRuntimePackAndAOTCompiler"
AfterTargets="ResolveFrameworkReferences"
Condition="'$(UseCurrentMainBranch)' == 'true'" >
<Error Condition="'$(DotnetRuntimeRepo)' == ''" Text="You must specify the location of your dotnet/runtime repository via -p:DotnetRuntimeRepo=<some_repo_dir>"/>
<ItemGroup>
<!-- update runtime pack to local build -->
<ResolvedRuntimePack PackageDirectory="$(DotnetRuntimeRepo)/artifacts/bin/microsoft.netcore.app.runtime.ios-arm64/Release"
Condition="'%(ResolvedRuntimePack.FrameworkName)' == 'Microsoft.NETCore.App'" />
<!-- remove AOT compiler reference for ios-arm64 -->
<MonoAotCrossCompiler Remove="@(MonoAotCrossCompiler)"
Condition="%(RuntimeIdentifier) == 'ios-arm64'" />
<!-- update AOT compiler reference to local build -->
<MonoAotCrossCompiler Include="$(DotnetRuntimeRepo)/artifacts/bin/mono/ios.arm64.Release/cross/ios-arm64/mono-aot-cross">
<RuntimeIdentifier>ios-arm64</RuntimeIdentifier>
</MonoAotCrossCompiler>
</ItemGroup>
</Target> and then build the app by passing in One thing to note is that the example is configured to target an iOS device. Hope this helps. |
…o_debug_lookup_source_location (#94612) Backport of #94540 to release/8.0-staging If a newly added method is in a stack trace, look it up in the EnC debug information, if available. Related to #93860 * Add new crashing test * [mono] catch the case of updated methods in mono_debug_lookup_source_location Co-authored-by: Aleksey Kliger <alklig@microsoft.com> Co-authored-by: Thays Grazia <thaystg@gmail.com>
@ivanpovazan that's very helpful. Unfortunately it only works with an actual device, not with a simulator (I adjusted the obvious RID-specific bits of the .csproj snippet). With the simulator I end up getting errors related to the sdb trampoline - which is usually an indication that some JITing is happening. @jeromelaban Does Uno hot reload work on an actual device? I tried running the app with Ctrl-F5 and making changes, but I didn't see anything show up. The |
The runtime pack should have a dotnet.native.js.symbols to symbolicate @radical can give you more details. You can also build a Debug runtime locally and update the runtime pack for browser-wasm to use that |
We have a symbolicator in
|
@lambdageek it is supposed to, yes, though because of this issue I wasn't able to test this very far. There may be error messages showing up in the device log, also. You may be able to see more messages when enabling trace logging here: builder.SetMinimumLevel(LogLevel.Trace); and here: builder.AddFilter("Uno.UI.RemoteControl", LogLevel.Trace); |
@jeromelaban Could you try .NET SDK 8.0.1 |
@lambdageek I'd like to, but there are no new available workloads available that I could see. VS 17.8.4 still has the 8.0.100 workloads installed. I tried with https://aka.ms/dotnet/maui/main.json, but those are causing install issues on macOS (not sure of the reason). Would you know of the workload versions I should be using for 8.0.101? |
@jeromelaban I don't really understand how workloads work. but with a clean install of 8.0.101 on osx-arm64if I run
which I think means it's getting the servicing versions of the runtime packs
@steveisok tells me the right place to look is |
This is what I see: % dotnet workload update --print-rollback
==workloadRollbackDefinitionJsonOutputStart==
{
"microsoft.net.sdk.android": "34.0.52/8.0.100",
"microsoft.net.sdk.ios": "17.0.8490/8.0.100",
"microsoft.net.sdk.maccatalyst": "17.0.8490/8.0.100",
"microsoft.net.sdk.macos": "14.0.8490/8.0.100",
"microsoft.net.sdk.maui": "8.0.3/8.0.100",
"microsoft.net.sdk.tvos": "17.0.8490/8.0.100",
"microsoft.net.workload.mono.toolchain.current": "8.0.1/8.0.100",
"microsoft.net.workload.emscripten.current": "8.0.1/8.0.100",
"microsoft.net.workload.emscripten.net6": "8.0.1/8.0.100",
"microsoft.net.workload.emscripten.net7": "8.0.1/8.0.100",
"microsoft.net.workload.mono.toolchain.net6": "8.0.1/8.0.100",
"microsoft.net.workload.mono.toolchain.net7": "8.0.1/8.0.100",
"microsoft.net.sdk.aspire": "8.0.0-preview.2.23619.3/8.0.100"
}
==workloadRollbackDefinitionJsonOutputEnd== maybe that's useful? |
@lambdageek yes, it's definitely useful, I did not know about Still, those were the versions I was using, and I'm having trouble with validating the upgrade from 8.0.100 to 8.0.101 (on macOS, the workload upgrade does not finish properly). Still that's out of the current issue, so I'll probably open a separate issue for this. I'll test HR for android/iOS and will report back here! Thanks for following up! |
So I've tested on android, and the issue is different now: After a few reloads (like two or three), I get this:
Interestingly, there's nothing in ADB that is recognizable, but I may not read it properly. So the current issue may be fixed in 8.0.101 for android but I can't test it... |
I'm also getting random crashes when starting the app with this:
But this is unrelated to hot reload. |
So the crash issue is #96872 (threading related), but if I work around it, I can confirm that the repro steps in #93860 (comment) are not failing anymore, thanks! |
Ok, the threading thing is because I don't read POSIX manpages closely enough. Closing this issue since the hot reload is working now. |
From @jeromelaban on Fri, 20 Oct 2023 03:53:31 GMT
Steps to Reproduce
[CreateNewOnMetadataUpdate]
attribute onMainPage
ToString();
call in the ctor, or any otherMainPage
modificationExpected Behavior
The code is updated without crashing.
Actual Behavior
Sometimes:
Sometimes:
Environment
Build Logs
Available on demand.
Copied from original issue xamarin/xamarin-macios#19277
The text was updated successfully, but these errors were encountered: