933654 - B2G RIL: PROCESS-CRASH on test_icc_card_state.js marionette test

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Emulator, defect)

Description

[Jessica Jong [:jessica] (inactive)]

Attachment: test_mobile_icc_change.js

The attaches file is the test case for Bug 814629. After adding the attached testcase, test_icc_card_state.js fails due to PROCESS-CRASH (abnormal termination with exit code -11) on try server. We will leave the attached test case out until the crash is solved.

The result of running the test cases separately is PASS, but FAIL when running the whole marionette test suite.

Reference try server run:
https://tbpl.mozilla.org/?tree=Try&rev=cf22c6c3b95b

Comment 1

[Vicamo Yang [:vicamo][:vyang]]

This case was first found when running all Marionette tests on emulator-jb without additional test cases installed.  100% reproduce-rate.

Comment 2

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Is Jessica or Vicamo already working on this?
If no, I'd like to let Georgia help to check this.

Comment 3

[Jessica Jong [:jessica] (inactive)]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #2)
> Is Jessica or Vicamo already working on this?
> If no, I'd like to let Georgia help to check this.

I am ok with it.
You don't need Bug 814629 to reproduce this, I've tried adding a marionette test under dom/network/tests/marionette/, which only does airplanemode on/off, and it can be reproduced...

Comment 4

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Georgia, can you help to discuss Jessica or Vicamo with this?

This error has been for a while, Vicamo has mentioned this last week.

Comment 5

[Georgia Wang]

I also can reproduce on emulator ICS when add a new test case & run all marionette.

Beside, I just found for current success marionette log, there's a strange warning when runing func "test_ril_card_state.js"...need further check.

GeckoConsole: [JavaScript Error: "NS_ERROR_FAILURE: Failure arg 0 [nsIDOMRequestService.fireSuccess]" {file: "jar:file:///system/b2g/omni.ja!/components/RILContentHelper.js" line: 1429}]

Comment 6

[Vicamo Yang [:vicamo][:vyang]]

If you don't mind :)

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Program received signal SIGSEGV, Segmentation fault.
sys_timer_set (timer=0x0, when=2270715259, _callback=0x4b2200 <radio_state_change_event>, opaque=0xc421e0 <_android_modem>) at external/qemu/telephony/sysdeps_qemu.c:111
111             if (timer->timer) {
(gdb) bt
#0  sys_timer_set (timer=0x0, when=2270715259, _callback=0x4b2200 <radio_state_change_event>, opaque=0xc421e0 <_android_modem>) at external/qemu/telephony/sysdeps_qemu.c:111
#1  0x00000000004b03a1 in handleRadioPower (modem=0xc421e0 <_android_modem>, cmd=<optimized out>) at external/qemu/telephony/android_modem.c:1685
#2  handleRadioPower (cmd=<optimized out>, modem=0xc421e0 <_android_modem>) at external/qemu/telephony/android_modem.c:1669

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

Looks like we're running out of free timer structs.  https://github.com/mozilla-b2g/platform_external_qemu/blob/master/telephony/sysdeps_qemu.c#L57

  #define  MAX_TIMERS  32

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Oops!  It seems all timers created by |handleRadioPower| are not freed.  :X

Comment 10

[Jessica Jong [:jessica] (inactive)]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #9)
> Oops!  It seems all timers created by |handleRadioPower| are not freed.  :X

sys_timer_destroy() missing!
oh~ how did I not noticed it! >.<

Comment 11

[Vicamo Yang [:vicamo][:vyang]]

rild expects radio_state changes right after AT+CFUNC calls so we have to modify |modem->radio_state| in the command handler and returns "OK" before any unsolicited response.  That's the reason we had set_timer thing in bug 877530.  However,  I think the ultimate solution is to allow extra unsolicited responses in command handlers so that we may skip these timer stuff, and I happen to have a patch [1] previously for bug 788928.  Just need its part 1.

[1]: https://github.com/mozilla-b2g/platform_external_qemu/pull/16

Comment 12

[Vicamo Yang [:vicamo][:vyang]]

Attachment: Github pull request for external/qemu

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

Attachment: [debug] test case

Since Jessica had provided her test script originally bug 814629, we can still land her patch once that GitHub PR is merged.

This patch is only for local verification.  It simply duplicates the existing test_icc_card_state.js as test_icc_card_state2.js.

Comment 14

[Vicamo Yang [:vicamo][:vyang]]

Verified on emulator-jb as well.

Comment 15

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 827292 [details]
Github pull request for external/qemu

Sorry for the delay. Please see my comments dropped on github.

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 827292 [details]
Github pull request for external/qemu

Replied on GitHub.

Comment 17

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 827292 [details]
Github pull request for external/qemu

Thanks for the explanation. r=me!

Comment 18

[Vicamo Yang [:vicamo][:vyang]]

master: https://github.com/vicamo/b2g_platform_external_qemu/commit/ea6018f85b5825e5185c17fa718fa0c8f6151244
b2g-jellybean: https://github.com/mozilla-b2g/platform_external_qemu/commit/11b1e54c534e7c548e43395a0140d97fee7274a5