Closed Bug 1007824 Opened 10 years ago Closed 10 years ago

Intermittent Shutdown | application crashed [@ ProfilerSignalHandler] after testANRReporter

Categories

(Firefox for Android Graveyard :: General, defect)

Product:
Firefox for Android Graveyard
Component:
General
Platform:
ARM
Android
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox31 unaffected, firefox32 fixed, firefox33 fixed, firefox-esr24 unaffected)

Status:
RESOLVED FIXED
Milestone:
Firefox 33
Tracking Flags:
Tracking Status
firefox31 --- unaffected
firefox32 --- fixed
firefox33 --- fixed
firefox-esr24 --- unaffected

People

(Reporter: RyanVM, Assigned: jchen)

Details

(Keywords: crash, intermittent-failure)

Attachments

(3 files)

Add mfbt/LinuxSignal.h (v1)
(deleted), patch
snorp
: review+
Ms2ger
: review+
Details | Diff | Splinter Review
Use signal trampoline for profiler signal handler (v1)
(deleted), patch
BenWa
: review+
Details | Diff | Splinter Review
Patch for uplifting
(deleted), patch
jchen
: review+
Sylvestre
: approval-mozilla-aurora+
Details | Diff | Splinter Review 
https://tbpl.mozilla.org/php/getParsedLog.php?id=39284340&tree=Mozilla-Inbound

Android 2.3 Emulator mozilla-inbound opt test robocop-1 on 2014-05-08 07:46:11 PDT for push cf209153f20e
slave: tst-linux64-spot-812

08:16:03     INFO -  0 INFO SimpleTest START
08:16:03     INFO -  1 INFO TEST-START | testANRReporter
08:16:03     INFO -  2 INFO TEST-PASS | testANRReporter | Given message occurred for registered event: {"type":"Gecko:Ready"} - Gecko:Ready should equal Gecko:Ready
08:16:03     INFO -  EventExpecter: no longer listening for Gecko:Ready
08:16:03     INFO -  3 INFO TEST-PASS | testANRReporter | Ping directory is empty
08:16:03     INFO -  4 INFO TEST-PASS | testANRReporter | Successfully set package name - org.mozilla.fennec should equal org.mozilla.fennec
08:16:03     INFO -  5 INFO TEST-PASS | testANRReporter | testContext should not be null - android.app.ContextImpl@4055fbc8 should not equal null
08:16:03     INFO -  6 INFO TEST-INFO | testANRReporter | Triggering ANR
08:16:03     INFO -  7 INFO TEST-INFO | testANRReporter | Triggering second ANR
08:16:03     INFO -  8 INFO TEST-INFO | testANRReporter | Waiting for ping
08:16:03     INFO -  9 INFO TEST-PASS | testANRReporter | Ping directory exists
08:16:03     INFO -  10 INFO TEST-PASS | testANRReporter | Ping directory is a directory
08:16:03     INFO -  11 INFO TEST-PASS | testANRReporter | Ping directory is not empty - [Ljava.io.File;@406d0f28 should not equal null
08:16:03     INFO -  12 INFO TEST-PASS | testANRReporter | ANR reporter wrote one ping - 1 should equal 1
08:16:03     INFO -  13 INFO TEST-PASS | testANRReporter | Ping exists
08:16:03     INFO -  14 INFO TEST-PASS | testANRReporter | Ping is a file
08:16:03     INFO -  15 INFO TEST-PASS | testANRReporter | Ping is readable
08:16:03     INFO -  16 INFO TEST-INFO | testANRReporter | Found ping file - /mnt/sdcard/tests/profile/saved-telemetry-pings/90aa1f8d-f519-40db-bb2c-c3cee3167b15
08:16:03     INFO -  17 INFO TEST-PASS | testANRReporter | Ping has slug property
08:16:03     INFO -  18 INFO TEST-PASS | testANRReporter | Ping has reason property
08:16:03     INFO -  19 INFO TEST-PASS | testANRReporter | Ping has payload property
08:16:03     INFO -  20 INFO TEST-PASS | testANRReporter | Payload has ver property
08:16:03     INFO -  21 INFO TEST-PASS | testANRReporter | Payload has info property
08:16:03     INFO -  22 INFO TEST-PASS | testANRReporter | Payload has androidANR property
08:16:03     INFO -  23 INFO TEST-PASS | testANRReporter | Info has reason property
08:16:03     INFO -  24 INFO TEST-PASS | testANRReporter | Info has appName property
08:16:03     INFO -  25 INFO TEST-PASS | testANRReporter | Info has appUpdateChannel property
08:16:03     INFO -  26 INFO TEST-PASS | testANRReporter | Info has appVersion property
08:16:03     INFO -  27 INFO TEST-PASS | testANRReporter | Info has appBuildID property
08:16:03     INFO -  28 INFO TEST-END | testANRReporter | finished in 33921ms
08:16:03     INFO -  29 INFO TEST-START | Shutdown
08:16:03     INFO -  30 INFO Passed: 22
08:16:03     INFO -  31 INFO Failed: 0
08:16:03     INFO -  32 INFO Todo: 0
08:16:03     INFO -  33 INFO SimpleTest FINISHED
08:16:03     INFO -  INFO | automation.py | Application ran for: 0:00:46.169962
08:16:03     INFO -  INFO | zombiecheck | Reading PID log: /tmp/tmpF3O8H8pidlog
08:16:03     INFO -  Contents of /data/anr/traces.txt:
08:16:03     INFO -  
08:16:03     INFO -  mozcrash INFO | Downloading symbols from: https://ftp-ssl.mozilla.org/pub/mozilla.org/mobile/tinderbox-builds/mozilla-inbound-android/1399557697/fennec-32.0a1.en-US.android-arm.crashreporter-symbols.zip
08:16:03  WARNING -  PROCESS-CRASH | Shutdown | application crashed [@ ProfilerSignalHandler]
08:16:03     INFO -  Crash dump filename: /tmp/tmpA44qOM/1c89f797-d6aa-5e5b-4946c863-2336c1a9.dmp
08:16:03     INFO -  Operating system: Android
08:16:03     INFO -                    0.0.0 Linux 2.6.29-ge3d684d #1 Mon Dec 16 22:26:51 UTC 2013 armv7l generic/sdk/generic:2.3.7/GINGERBREAD/eng.ubuntu.20140123.014351:eng/test-keys
08:16:03     INFO -  CPU: arm
08:16:03     INFO -       0 CPUs
08:16:03     INFO -  
08:16:03     INFO -  Crash reason:  SIGSEGV
08:16:03     INFO -  Crash address: 0x23920611
08:16:03     INFO -  
08:16:03     INFO -  Thread 12 (crashed)
08:16:03     INFO -   0  libxul.so!ProfilerSignalHandler [platform-linux.cc:cf209153f20e : 220 + 0x0]
08:16:03     INFO -       r4 = 0x23920601    r5 = 0x51c031b8    r6 = 0x00000000    r7 = 0x961a1fc6
08:16:03     INFO -       r8 = 0x00000012    r9 = 0x4de5204d   r10 = 0x47d6038c    fp = 0x00000001
08:16:03     INFO -       sp = 0x47d5ff28    lr = 0xffff0515    pc = 0x4e34fd0a
08:16:03     INFO -      Found by: given as instruction pointer in context
08:16:03     INFO -   1  0xffff0513
08:16:03     INFO -       r4 = 0x51de6d68    r5 = 0x51c031b8    r6 = 0x00000000    r7 = 0x961a1fc6
08:16:03     INFO -       r8 = 0x00000012    r9 = 0x4de5204d   r10 = 0x47d6038c    fp = 0x00000001
08:16:03     INFO -       sp = 0x47d5ff60    pc = 0xffff0515
08:16:03     INFO -      Found by: call frame info
I bet it has the same cause as bug 985155.
Attached patch Add mfbt/LinuxSignal.h (v1) (deleted) — — Splinter Review 
To fix the crash (see bug 985155 comment 49), we need a small trampoline on ARM for our signal handlers to avoid the kernel bug. I think the best place to put the trampoline code is MFBT because we'll potentially be using this trampoline in both js and xul.

LinuxSignal.h can also (in another bug) be used to consolidate our real-time signal number allocation (if you search for SIGRTMIN, there are several different places where we allocate real-time signals, so it'd be good to have a central location for these allocations).
Attachment #8453982 - Flags: review?(Ms2ger)
This makes the profiler use the signal trampoline on ARM to workaround the kernel bug in our Android 2.3 emulator.
Attachment #8453986 - Flags: review?(bgirard)
Attachment #8453986 - Flags: review?(bgirard) → review+
Comment on attachment 8453982 [details] [diff] [review]
Add mfbt/LinuxSignal.h (v1)

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

You'll have to find someone who understands what you're doing and why; feel free to ask me to rubber-stamp the addition when you do :)
Attachment #8453982 - Flags: review?(Ms2ger)
Comment on attachment 8453982 [details] [diff] [review]
Add mfbt/LinuxSignal.h (v1)

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

r? per comment 58
Attachment #8453982 - Flags: review?(snorp)
(In reply to :Ms2ger from comment #58)
> Comment on attachment 8453982 [details] [diff] [review]
> Add mfbt/LinuxSignal.h (v1)
> 
> Review of attachment 8453982 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> You'll have to find someone who understands what you're doing and why; feel
> free to ask me to rubber-stamp the addition when you do :)

This is the same fix we had for bug 985155. It was removed in bug 1016441 because I guess Jim thought the bug wouldn't be present there.
Attachment #8453982 - Flags: review?(snorp) → review+
Comment on attachment 8453982 [details] [diff] [review]
Add mfbt/LinuxSignal.h (v1)

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

I think we're good now? :)
Attachment #8453982 - Flags: review?(Ms2ger)
Comment on attachment 8453982 [details] [diff] [review]
Add mfbt/LinuxSignal.h (v1)

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

r=me with those two nits addressed

::: mfbt/LinuxSignal.h
@@ +4,5 @@
> +
> +#ifndef mozilla_LinuxSignal_h
> +#define mozilla_LinuxSignal_h
> +
> +namespace mozilla

{ should be on this line

@@ +8,5 @@
> +namespace mozilla
> +{
> +
> +#if defined(__arm__)
> +  // Some (old) Linux kernels on ARM have a bug where a signal handler

Don't indent the contents of a namespace block
Attachment #8453982 - Flags: review?(Ms2ger) → review+
https://hg.mozilla.org/mozilla-central/rev/2025d6c50499
https://hg.mozilla.org/mozilla-central/rev/68128a705799
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 33
Please request Aurora approval for this too.
status-firefox31: --- → unaffected
status-firefox32: --- → affected
status-firefox33: --- → fixed
status-firefox-esr24: --- → unaffected
Flags: needinfo?(nchen)
Comment on attachment 8453982 [details] [diff] [review]
Add mfbt/LinuxSignal.h (v1)

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Fixes intermittent crash on TBPL and possible crash whenever profiler is active.
[Describe test coverage new/current, TBPL]: Covered by Robocop test on Android.
[Risks and why]: Very small because fix only addresses crash on ARM builds.
[String/UUID change made/needed]: None
Attachment #8453982 - Flags: approval-mozilla-aurora?
Flags: needinfo?(nchen)
Attached patch Patch for uplifting (deleted) — — Splinter Review 

        Attachment #8458045 -
        Flags: review+

        Attachment #8453982 -
        Flags: approval-mozilla-aurora?

    
    

    
  
Comment on attachment 8458045 [details] [diff] [review]
Patch for uplifting

Approval Request Comment
[Feature/regressing bug #]: N/A
[User impact if declined]: Fixes intermittent crash on TBPL and possible crash whenever profiler is active.
[Describe test coverage new/current, TBPL]: Covered by Robocop test on Android.
[Risks and why]: Very small because fix only addresses crash on ARM builds.
[String/UUID change made/needed]: None

        Attachment #8458045 -
        Flags: approval-mozilla-aurora?

    
    

    
      
  

        Attachment #8458045 -
        Flags: approval-mozilla-aurora? → approval-mozilla-aurora+

    
    

    
      
  
Product: Firefox for Android → Firefox for Android Graveyard

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: