1162812 - VibratorRunnable prevents Nuwa from freezing

Status: RESOLVED WORKSFORME

(Core :: Hardware Abstraction Layer (HAL), defect)

Description

[Ben Turner (not reading bugmail, use the needinfo flag!)]

VibratorRunnable prevents Nuwa from freezing. http://mxr.mozilla.org/mozilla-central/source/hal/gonk/GonkHal.cpp#348 blocks forever, potentially, and our idle thread detection code thinks that the thread is always working, so it never tells the Nuwa process to freeze.

Comment 1

[Gregor Wagner [:gwagner]]

We need to figure this out for 2.2.
Cervantes, can you take care of it?

Comment 2

[Cervantes Yu [:cyu] [:cervantes]]

VibratorRunnable is created the first time navigator.vibrate is called. There is a theoretical race between the Nuwa process and the vibrator, but in practice it's unlikely that the system app has the chance to request vibration before Nuwa is ready.

The more practical concern is that after the first vibration, if Nuwa is ever killed, the restarted Nuwa process won't be ready because the vibrator thread is considered always working.

Comment 3

[Ben Turner (not reading bugmail, use the needinfo flag!)]

(In reply to Cervantes Yu from comment #2)
> VibratorRunnable is created the first time navigator.vibrate is called.
> There is a theoretical race between the Nuwa process and the vibrator, but
> in practice it's unlikely that the system app has the chance to request
> vibration before Nuwa is ready.

This is what we are seeing in trunk though.

Comment 4

[Gregor Wagner [:gwagner]]

It comes somewhere from /apps/system/js/sound_manager.js
Do we really need to call vibrate during startup here?

Comment 5

[Gregor Wagner [:gwagner]]

Seems like this push broke all our nuwa idle detection:
https://github.com/mozilla-b2g/gaia/pull/29900/files

Comment 6

[John Hu [:johnhu][:johu][:醬糊小弟]]

(In reply to Gregor Wagner [:gwagner] from comment #4)
> It comes somewhere from /apps/system/js/sound_manager.js
> Do we really need to call vibrate during startup here?

Sorry, I don't know why we need to vibrate during startup. At the moment I rewrote the sound manager, we didn't need to do it.

And according to the code (PR 29900), it doesn't make sense to vibrate while settings changed[1]. If the original patch tries to vibrate while user changes volume to vibrate, we already did that at [2][3].

[1] https://github.com/mozilla-b2g/gaia/blob/e0ef9a3235daea127e4860e153891f98e5896554/apps/system/js/sound_manager.js#L503-L507
[2] https://github.com/mozilla-b2g/gaia/blob/226eea52e46c931150aa26ad015bda91d83c4127/apps/system/js/sound_manager.js#L960-L962
[3] https://github.com/mozilla-b2g/gaia/blob/226eea52e46c931150aa26ad015bda91d83c4127/apps/system/js/sound_manager.js#L838-L840

Comment 7

[Josh Cheng [:josh]]

Hi, 
Assume we are not blocking here.
Do we need more discussion to decide whether to call vibrate during startup?

Comment 8

[Bobby Chien]

unblocking v2.2 per bug 1151672.

Comment 9

[Bobby Chien]

According to comment 8 and code backout in 2.2, this issue doesn't exist in v2.2.