-
Notifications
You must be signed in to change notification settings - Fork 539
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
Increased amount of ANRs after disabling concurrent GC #9365
Comments
@TimBurik I'm afraid we can't do anything for you here, this appears to be an issue split between MonoVM and the Sentry Native SDK. @lambdageek do you think someone on your side could take a look to see if anything can be done here? There's an issue being worked on related to Sentry, #9055, perhaps this here issue is also caused by the same problems? @supervacuus, sorry for tagging you out of the blue, but would you be able to look into the Sentry side of things here? I realize we have precious little information, but perhaps someone will be able to spot something in their respective areas and help out. |
@grendello thank you for the response. Regarding #9055 - we are aware of this issue, and we have a separate native crash group, which is most likely related to this issue. But the ANRs doesn't seem to be related to this issue and to Sentry, because we are seeing exactly the same picture in the GooglePlay Console - increased amount of ANRs, all containing |
The GC ANRs might be related to something inside the GC bridge, hopefully @lambdageek will be able to help out here. However, this ANR looks familiar:
You are most likely not using marshal methods, since they weren't enabled by default in NET8, but we've fixed an issue recently which was related to them and one of the repros had the above stack trace as well. The problem was related to Java ("native") threads being incorrectly attached to the MonoVM runtime and putting the GC in a bad state. You can find more info about it in this and the following comments. @filipnavara, does this look familiar? Perhaps something in your app causes a similar corruption of the GC state? @TimBurik if you're able to reproduce the ANRs locally, would you be able to test with NET9 rc2? |
@grendello thank you for the pointers! We are planning to test the app on .NET 9 in the nearest future, we would make sure to use RC2 for those tests (when RC2 would be available). Also, we have added debug symbols for libmonosgen-2.0 into GooglePlay Console and now we have more detailed logs. For example, this is the reported main thread stack trace from one of the ANRs:
|
@TimBurik this more and more looks like the issue fixed by @filipnavara, he declared he'd take a look next week - it would be great if you were able to test with NET9 RC2 before then :) |
@TimBurik if RC2 isn't available by next week, do try with RC1 - it's to see if the issues you're seeing still even exist in NET9 |
I've been following the thread since you tagged me. It seems this isn't sentry-related. I've informed sentry peeps, just in case. |
@supervacuus thanks, I tagged you just in case - this is all low-level stuff, it might be affecting Sentry as well in some ways. |
/cc @vitek-karas |
@grendello sorry for late response
|
I don't know how the ANR system works, I would imagine they happen if the app is non responsive for a while. If this is the case, I'm not sure whether this issue can actually be considered to be a bug. We seem to have a problem with the concurrent GC in the runtime and this issue needs to be investigated somehow. Disabling concurrent GC will just lead to longer GC pause times which seem to trigger this ANR. Is a GC pause time of 0.5 - 1s supposed to trigger this ANR ? If this is true, it seems to me that the best way to move forward would be to share the application so we can see if the serial GC takes longer than expected for whatever reason and mainly to try some debug flags to see if we detect some problems with the concurrent collector. |
@BrzVlad according to the Android documentation, one of the reasons for ANR is when BroadcastReceiver has failed to process an event within 5 seconds - this seems to be our case, as there's an additional description in the GooglePlay Console like "Broadcast of Intent { act=android.intent.action.SCREEN_ON }" By the way, let me share a full report from the GooglePlay Console, which contains stack traces from all the threads: anr_all_stacktraces.txt I could also try to gather additional information regarding the GC performance, but in general we didn't notice any significant performance degradation after disabling concurrent GC |
@grendello in our case most of the ANRs are related to the But this is not the only source for ANRs for us, as we also have reports:
All of those reports have very similar main thread stack traces, related to GC. It's even seems that those broadcast events, service calls and input events are not really causing the issue, and the main thread is already blocked by the time these events occur. |
@grendello @BrzVlad is there anything else we could do to help you shed the light on this issue? |
Seing that this issue happens after disabling concurrent GC I was biased into assuming that the GCs simply take longer to run and this is causing issues. After a second look at the stacktraces, it indeed looks like the suspend machinery is hanging. It makes no sense to me why this issue would only happen with serial GC though. We have an env var |
@BrzVlad could you also give a hint to where the threads dump could be found? In logcat we see the message: We are using the following values as Android environment variables:
and also we are using the |
I would have expected that the suspend state of each thread to be printed but it looks like that logging path doesn't go through the android log but rather to the console. It would be useful for us to try to reproduce this failure but given the abort limit is not excessive and it only happens while in background, it makes me wonder whether it just the os scheduling the app less frequently and the suspend process just taking a bit longer than usual, because threads reach GC safepoint location slower. |
This issue doesn't seem to affect every GC in the background, in the log cat we also see a couple of GC-related logs:
I'm also gonna try to reproduce this issue with bigger values for |
@BrzVlad Just managed to reproduce the issue with limit value of 1s:
The reproduction steps are the same - put application to the background and wait, although this time it required also several tries (foregrounding/bacgrounding app back) and considerable amount of waiting (30m+ after each backgrounding) |
Would it be possible to provide the sample app with some reproduction steps ? |
Let me try to reproduce the issue on the sample app, and also discuss with the legal team possibility to share our production code. Meanwhile, I have found a similar issue with a deadlock in GC stopping the world mono/mono#21582, which ended up being fixed by changing the value of the |
That is a good observation. The default suspend mode should by |
@BrzVlad We have just tested
The bad news is that instead, we now see a brand new ANR groups, related to
I'm also attaching full stacktraces just in case: |
@TimBurik, having threads blocking in
When runtime restart threads, it will issue SIGXCPU that will complete It is critical that our suspend handlers are the handlers registered for these signals for the lifetime of the process. If something alters that after the runtime has loaded, newly created threads won't have the right signal handlers and if they attach to the runtime, those threads will either block stw to suspend the thrad (won't react to our SIGPWR signal) or prevent us from resuming the thread (won't react to our SIGXCPU signal). Do you use any native or C#/Java library inside this application that potentially could alter our signal handlers in anyway? I have looked a several of the attached anr dump files in this issue, but they all seem to be truncated ending right in the middle of a threads callstack. For example signal__anr_full2.txt ends like this when I download it:
Is that something you see in the original files as well or do you have more content? |
@lateralusX we are using sentry-dotnet and google-breakpad for crash analytics, which both seems to be altering SIGXCPU signal handler: And regarding the callstacks - this is everything that we see in GooglePlay Console, the only alteration to the original reports we did is obfuscating the package name) |
I made a fix for dotnet/runtime#111485 that solves the deadlock that issue end up with using hybrid suspend. Coop and Android might have the same issues as you see when running hybrid, because looking at your callstacks from preemptive suspend I see that there are Mono threads blocked on Java thread pools that we have suspend (using signals), if the same threads attach to Mono runtime under hybrid/coop suspend, then they must have been attached in a way that is hybrid/coop compatible or those threads have violated Mono's threading model ending up in RUNNING state while blocking outside of managed or Mono runtime code. In the past (Xamarin) Android ran using preemptive suspend model, so it would be good to make sure our signal handlers are not disabled by any 3'rd party code in this app. I know another library that mess with SIGXCPU, FFMPEG, but maybe you don't use that library in this app. |
@lateralusX we got the result of our test with disabled crash reporters, and according to GooglePlay Console the total amount of ANRs and the amount of user-perceived ANRs has been halved comparing to previous release! It seems to confirm the theory that overriding signal handlers is the root cause of the ANRs. But also, interestingly enough, it did not eliminate ANR_suspend_signal_handler_full1.txt Although disabling crash reporters does reduce the amount of ANRs, it leaves us with very limited options for monitoring app health, especially in terms of native crashes. Is there a way to monitor native crashes without interfering with dotnet's signal handlers? In Xamarin, we used to call |
UPD: after a longer observation we see that ANR rate has increased and stabilized on the value, very similar to the value without disabling crash reporting. At the moment it is ~20% improvement, instead of ~50% we have observed initially. Still though, if you have any useful tips or insights for signal handlers usage and native crash monitoring - we could test them out. |
Re: "In Xamarin, we used to call Mono.Runtime.RemoveSignalHandlers()/Mono.Runtime.InstallSignalHandlers() around initialization of the Breakpad, this is probably why Xamarin is not affected by using native reporter. Is there an alternative to this API in dotnet?" That part was never ported over to dotnet/runtime, see this for more discussions, dotnet/runtime#79706 and then there is a proposal here dotnet/runtime#101560 to get hold of unhandled exceptions and fatal errors without relying on overriding signals. Issue with that is that it needs 3'rd party libraries to support it and it won't handle all signals that runtime needs control of. The underlying implementation of the InstallSignalHandlers/RemoveSignalHandlers in mono/mono can be found here, https://github.com/mono/mono/blob/0f53e9e151d92944cacab3e24ac359410c606df6/mono/mini/mini-posix.c#L451 and here, https://github.com/mono/mono/blob/0f53e9e151d92944cacab3e24ac359410c606df6/mono/mini/mini-posix.c#L509. For dotnet/runtime mono what is most interesting in light of this issue are the signal handler's setup to handle thread suspend/resume, https://github.com/dotnet/runtime/blob/a9b700a1f6f3f524595393433bda6115b39afd71/src/mono/mono/utils/mono-threads-posix-signals.c#L227. For the thread specific signals, we won't chain them since we assume we have exclusive access to those signals. An unsupported, none portable way is to at least make sure the signals installed in |
@TimBurik I also looked at your latest ANR dumps: ANR_suspend_signal_handler_full1.txt: This one seems to be truncated so there are threads cut out but I it still looks like we hit a rather interesting scenario. Main thread does some work that will hold loader lock, taken in mono_class_vtable_checked. Then there is "Thread-4" that runs finalizers and does a stackwalk, but it does a suspend and run to do the stackwalk, so I assume it has suspended Main thread while holding the loader lock. Then as part of the stackwalk it hits a case where it needs to create a class instance, that needs the loader lock, leading to a deadlock. The only way to request a thread dump that will trigger the stackwalk is to explicit call mono_threads_request_thread_dump API. In mono that happens only in the SIGQUIT handler, so that signal seems to been fired and this is a side effect leading up to the deadlock. I can probably fix this issue by making sure the stackwalk done in response to mono_threads_request_thread_dump is async safe since its designed to be called from within signal handlers, so it expects to only do signal safe operations, meaning it won't try to load additional classes if not already loaded etc. Since this is a side effect of a SIGQUIT signal, it will of course not tackle why that happens, but in the dump no other threads seems to be doing anything strange. ANR_suspend_signal_handler_full2.txt: This one is slightly different, but it ends up in similar deadlock. In this case we have thread "AndroidAnrWatchDog" that calls into Mono runtime to get a type name, that leads up to a SIGSEGV (probably due to a heap corruption), that will call our SIGSEGV handler that does a native crash, triggering a full stackwalk of crashing thread. In parallel there is code triggering a GC, "Thread-14", it holds GC lock and wait for threads to suspend, another thread "GLThread 1071" gets suspended while holding loader lock and the thread handling the SIGSEGV is about stackwalk, but doesn't do it async safe, leading up to a deadlock on loader lock, so it deadlocks inside the signal handler, meaning that the suspend signal also send to this thread will wait, meaning that the GC thread will wait forever on this thread. Making sure we do async safe stackwalk from within crash handler would fix this chain of events, but it won't fix the root cause that is probably a heap corruption, but at least it should make it into a crash report intsead of an ANR. With these two dumps I have a couple of things to look at, it won't solve the underlying triggers (SIGQUIT and heap corruption) but it will at least make sure we would handle this complex error reporting scenarios better. |
Mono's stack-walker can be signal/async safe when needed, making sure it won't allocate additional memory or taking internal runtime locks. It is however up to the caller of the stack walking API's to decide if it should be signal and/or async safe. Currently this is controlled using two different flags, the MONO_UNWIND_SIGNAL_SAFE as well as mono_thread_info_is_async_context. This is problematic since callers wants signal safe stack-walking but since not both are set, it will not behave fully signal safe. dotnet/android#9365 hit a couple of scenarios described here: dotnet/android#9365 (comment) that ends up deadlocking due to the fact that they did stack-walking from within a signal handler and deadlocked or dumping stack on suspended thread holding runtime loader lock, but without making the stack-walk async safe. Fix makes sure that calls to stack-walk API's can be made signal and/or async safe and that identified areas uses the correct set of flags given state of threads when stack-walking.
Following PR should at least fix deadlocks in above ANR's, dotnet/runtime#113645. |
@TimBurik I looked back in history of this issue and realized that you had a local repro of the issue? Is that correct? If so, would it be possible for you to build a local runtime with some additional instrumentation (logging additional data into logcat) and use that together with your repro app? That way we could potentially get an idea around what thread that doesn't seem to suspend and then we could map that with the ANR to get what thread(s) we are waiting on. We could first try this with the original hybrid suspend mode and then potentially with preemptive suspend (with above fix) for even more additional information. |
@lateralusX Thank you very much for taking your time to look into the dumps!
This is actually sounds great! We were suspecting that some of those ANRs are happening while processing crashes (because of the high amount of "background" ANRs in GooglePlay Console). We would very much prefer those crashes reported as crashes, and not as ANRs, as we currently have disproportionally high ANR rate comparing to the crash rate in production.
This one seems to be actually related to another ticket of ours related to crashes: #9563 It seems that our custom ANR monitor implementation ("AndroidAnrWatchDog" thread) was hitting some kind of infinite loop while collecting the context, requesting type name information over and over and over again, probably causing heap corruption in the end. Either way, after disabling this functionality the crash group has gone, and related ANRs probably as well.
Yes, we do have a somewhat stable reproduction of the GC deadlock with a precise configuration:
Sure! Do you have any specific ideas in mind of what instrumentation we could add/turn-on in the runtime? We would be able to run the test some time next week. |
@TimBurik Sounds great. Let me write up some instructions during the week and share. Are you just using an Android SDK app (dotnet new android) or MAUI (dotnet new maui)? You will need to do a local build of the runtime enabling some additional logging in code, then you will use that version of the runtime when building your app running the reproduction step. The additional logging will hit logcat, so we will need to get some data out of logcat once the ANR reproduces, it will tell us more details on the threads that we are waiting on as part of the STW (Stop The World). When we have that data we can look in the ANR to detect what thread seems to be in wrong state and why it won't seem to reach a safe point in a timely manner. |
Android framework version
net8.0-android
Affected platform version
.NET 8.0.303
Description
After switching from Xamarin.Android to .Net8 we used to get a lot of native crashes in monosgen, for example 222003f52fa3496385d14a89c778a6e4-symbolicated.txt
After long investigation (and a hint from the issue dotnet/runtime#100311) It turns out that concurrent SGen is actually enabled by default in .net-android
android/src/Xamarin.Android.Build.Tasks/Microsoft.Android.Sdk/targets/Microsoft.Android.Sdk.DefaultProperties.targets
Line 9 in df9111d
so we explicitly disable it - and now the amount of native crashes in monosgen is minimal, but instead we are getting a lot of ANR reports in Sentry and GooglePlay Console.
ANRs seems to be reported using Android's ApplicationExitInfo mechanism, according to stacktraces main thread seems to be blocked by awaiting native mutex somewhere in the monosgen (example: anr_stacktrace.txt)
Additional information, which might be relevant:
Intent { act=android.intent.action.SCREEN_OFF }
orIntent { act=android.intent.action.SCREEN_ON }
;Steps to Reproduce
Unfortunately, we don't have exact steps to reproduce.
The only thing that is sure that it is happening when targeting
.net-android34.0
(version for Xamarin.Android doesn't have this issue) and issue started happening after adding the following to the csproj:<AndroidEnableSGenConcurrent>false</AndroidEnableSGenConcurrent>
Did you find any workaround?
No workaround found yet
Relevant log output
No response
The text was updated successfully, but these errors were encountered: