Closed Bug 929139 Opened 6 years ago Closed 6 years ago

Debug emulators crash on startup: MOZ_Assert: Assertion failure: mState != hal::SWITCH_STATE_UNKNOWN, at ../../../gecko/dom/system/gonk/AudioChannelManager.h:52

Categories

(Firefox OS Graveyard :: AudioChannel, defect, major)

defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jgriffin, Assigned: mchen)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

Debug B2G emulators have started crashing on startup with this error:

10:24:05     INFO -  10-21 17:24:04.690    45    45 F MOZ_Assert: Assertion failure: mState != hal::SWITCH_STATE_UNKNOWN, at ../../../gecko/dom/system/gonk/AudioChannelManager.h:52
10:24:05    ERROR -  10-21 17:24:04.690    45    45 F libc    : Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1)

This is preventing any debug tests from running.
full log:  https://tbpl.mozilla.org/php/getParsedLog.php?id=29445946&tree=Cedar&full=1

Andrew, can you get someone to take a look at this?  This completely blocks all work regarding getting debug tests running on TBPL.
Flags: needinfo?(overholt)
Marco, looks like the asserts you added as part of bug 889730 are causing big issues with test automation debug builds.  Can you please investigate?  Thanks!
Assignee: nobody → mchen
Flags: needinfo?(overholt) → needinfo?(mchen)
I will take a look at it but it is strange of that patch was landed on July but got failed on October.
Flags: needinfo?(mchen)
Ok, recently Gaia landed a patch for grabbing the headset status during initial step of system app.
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/statusbar.js#L183

If there is no headset during booting device up, then the mState of AudioChannelManager will be "unknow" state (it's a initial value). Finally it hits the assert. Since this assert is used to prevent switch hal tried to send unknow state (illegal value), so I would like to keep it there but change the initial state to OFF state.

Sorry for making inconvenient to TPBL. Will update patch later.
After testing on real device and dig into the code, AudioChannelManager will ask the SwitchHAL for headset's current status in it's constructor code so there is no chance that state will be unknown then trigger that assert. 

Try to build emulator version, maybe emulator version missed the uevent path so it's state will be unknown. Will check it.
Attached patch Patch v1 (obsolete) — Splinter Review
Yes, confirm that the kernel in emulator build didn't have /sys/devices/virtual/switch folder so the state will be reported as unknown by GonkSwitch.

Consider that some target devices (ex: emulator, TV) may not have such kind of sys node to support headset switch state. So 
  1. Remove the assert check.
  2. Add the condition to treat unknown as "no headset".
Attachment #820347 - Flags: review?(amarchesini)
FYI:  baku's on PTO until October 28th.
Comment on attachment 820347 [details] [diff] [review]
Patch v1

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

::: dom/system/gonk/AudioChannelManager.h
@@ +51,5 @@
>    {
> +    // Bug 929139 - Remove the assert check for SWITCH_STATE_UNKNOWN.
> +    // If any devices (ex: emulator) didn't have the corresponding sys node for
> +    // headset switch state then GonkSwitch will report the unknown state. So
> +    // it is possible of getting unknown state here.

the english is messed up here.  Maybe it should read:

   it is possible to get unknown state here?

@@ +53,5 @@
> +    // If any devices (ex: emulator) didn't have the corresponding sys node for
> +    // headset switch state then GonkSwitch will report the unknown state. So
> +    // it is possible of getting unknown state here.
> +    return mState != hal::SWITCH_STATE_OFF &&
> +           mState != hal::SWITCH_STATE_UNKNOWN;

Why not test explicitly for:

 SWITCH_STATE_HEADSET,          // Headphone with microphone
 SWITCH_STATE_HEADPHONE,        // without microphone
Attachment #820347 - Flags: review?(amarchesini) → review?(doug.turner)
Oops, didn't see Doug's comment first.
Component: DOM: Device Interfaces → AudioChannel
Product: Core → Boot2Gecko
Version: Trunk → unspecified
Attached patch Patch v2 (obsolete) — Splinter Review
(In reply to Doug Turner (:dougt) from comment #8)
>    it is possible to get unknown state here?

Follow the comment.

>
> > +    // it is possible of getting unknown state here.
> > +    return mState != hal::SWITCH_STATE_OFF &&
> > +           mState != hal::SWITCH_STATE_UNKNOWN;
> 
> Why not test explicitly for:
> 
>  SWITCH_STATE_HEADSET,          // Headphone with microphone
>  SWITCH_STATE_HEADPHONE,        // without microphone

Consider that FxOS may add more headset types (ex: USB headset) in the feature, so I choose to check the non-headset states which conditions equals to headset types and less then headset types in the future. In this design we don't need to add more check here when new headset type is added.
Attachment #820347 - Attachment is obsolete: true
Attachment #820347 - Flags: review?(doug.turner)
Attachment #820831 - Flags: review?(doug.turner)
Comment on attachment 820831 [details] [diff] [review]
Patch v2

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

lgtm
Attachment #820831 - Flags: review?(doug.turner) → review+
Carry reviewer name.
Attachment #820831 - Attachment is obsolete: true
Attachment #822132 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7b130ccf80f4
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
You need to log in before you can comment on or make changes to this bug.