959214 - Use unwinder when getting ANR native stack

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[(inactive) Jim Chen [:jchen] [:darchons]]

We need the Gecko thread stack to fix a majority of ANRs. So far the ANR native stack only includes the pseudostack, which limits its effectiveness. We can use the unwinder to get a better native stack.

Comment 1

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Use unwinder when getting ANR native stack (v1)

This patch turns on unwinding for ANR native stacks when running on high-memory devices (due to unwinding being somewhat memory-intensive). I also limited native stacks to the main thread and Compositor thread to make sure we can finish in reasonable time, before the user kills us.

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 8360205 [details] [diff] [review]
Use unwinder when getting ANR native stack (v1)

Review of attachment 8360205 [details] [diff] [review]:
-----------------------------------------------------------------

::: widget/android/AndroidJNI.cpp
@@ +970,5 @@
> +    if (aUnwind) {
> +        features = NATIVE_STACK_UNWIND_FEATURES;
> +        features_size = sizeof(NATIVE_STACK_UNWIND_FEATURES);
> +        // We want the new unwinder if the unwind mode has not been set yet
> +        putenv("MOZ_PROFILER_NEW=1");

What is the effect of this? It won't get unset if this is called with aUnwind = false later.

Comment 3

[(inactive) Jim Chen [:jchen] [:darchons]]

Attachment: Use open/mmap for libmozglue unwinding (v1)

> Too hackish for my taste. I'd rather see ReadSymbolData_ANDROID use plain
> open/mmap/close for libmozglue.so instead, until the linker has the right
> support to make this unnecessary (which is not trivial, but necessary for
> other reasons anyways, so it will happen some day).

Okay. It's hackish either way :)  But here's the patch to use open/mmap.

Comment 4

[(inactive) Jim Chen [:jchen] [:darchons]]

Comment on attachment 8367417 [details] [diff] [review]
Use open/mmap for libmozglue unwinding (v1)

Oops wrong bug :)

Comment 5

[(inactive) Jim Chen [:jchen] [:darchons]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/2414def6feab

(In reply to Brad Lassey [:blassey] (use needinfo?) from comment #2)
> Comment on attachment 8360205 [details] [diff] [review]
> Use unwinder when getting ANR native stack (v1)
> 
> Review of attachment 8360205 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: widget/android/AndroidJNI.cpp
> @@ +970,5 @@
> > +    if (aUnwind) {
> > +        features = NATIVE_STACK_UNWIND_FEATURES;
> > +        features_size = sizeof(NATIVE_STACK_UNWIND_FEATURES);
> > +        // We want the new unwinder if the unwind mode has not been set yet
> > +        putenv("MOZ_PROFILER_NEW=1");
> 
> What is the effect of this? It won't get unset if this is called with
> aUnwind = false later.

I don't think it's a concern because this env var is only used when we ask the profiler to unwind.

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/2414def6feab