986753 - [Tarako] Alarm Clock can get killed in the background and never ring

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Clock, defect)

Description

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

1. launch alarm clock
2. set alarm for 2 minutes into the future with a ring
3. launch camera
4. take a picture
5. switch to gallery
6. open the picture
7. edit the picture, crop, sepia, effects
8. save the picture
9. repeat 6 to 8 until past the time

Expected: the alarm rings on time
Actual: the alarm doesn't ring

Gaia      63a7f183788218871c95a887328d091d58519edf
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/818e38fe5f4d
BuildID   20140321101803
Version   28.0
ro.build.version.incremental=86
ro.build.date=Thu Mar 20 11:25:47 CST 2014
Tarako

Note:
1. sometimes after you reboot the alarm might go off with the time that you had set it to.

Comment 1

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

noming because the purpose of the alarm fails to function.

Comment 2

[Joe Cheng [:jcheng] (please needinfo)]

traige: 1.3T+ for failing functinoality 
ni? Dylan, do you mind having someone to look at it? Thanks

Comment 3

[Dylan Oliver [:doliver]]

cc: bkelly

Marcus, can you work with Ben on this one to learn some of the perf team's tricks of the trade on handling Tarako low memory situations?

Comment 4

[Marcus Cavanaugh [:mcav]]

I can ping :bkelly, though this particular bug looks more like a clock alarm-firing bug than anything else (unless Clock is somehow getting killed when it's trying to fire the alarm, though that seems unlikely). Will look into it.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Yea, I'd say figuring out if the alarm is trying to fire or not would be the first step.  Do we use a hardware clock or something to get notified about the alarm?  I wonder if the firmware bits are there or not.

Please let me know if I can help with any general memory or perf investigation questions.

Comment 6

[Marcus Cavanaugh [:mcav]]

Attachment: Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17587

In the voice of Monk, here's what happened:

In super-high-memory-pressure situations, it may take a long time for the pressure to be relieved enough for Clock's alarm screen to boot up (load modules, etc). The module loader was set to timeout after 7 seconds; if it took longer than that to load the modules, the loader would timeout and the alarm would never display.

In manual testing, upping the timeout value caused the alarm to properly fire when the phone caught up with memory use. In this patch, I set the limit at 3 minutes -- but it really could be arbitrarily high. I'm having :jrburke review this, in case he has any thoughts on setting an even higher value.

Comment 7

[James Burke [:jrburke]]

Comment on attachment 8396458 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17587

I added some info to the pull request, feel free to comment or update the pull request an flip it back to me for review.

Comment 8

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8396458 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17587

Yes, your suggestion is much better. I've revised it accordingly and verified that waitSeconds actually does get set to 0 in the app zip after `make`.

Comment 9

[Marcus Cavanaugh [:mcav]]

Landed in master:

https://github.com/mozilla-b2g/gaia/commit/ede2b2488f467a0b8bddf5a741e07464cd15f735

Comment 10

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8396458 [details] [review]
Link to Github pull-request: https://github.com/mozilla-b2g/gaia/pull/17587

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): n/a
[User impact] if declined: Alarms may fail to fire if the phone is under severe memory pressure at alarm time.
[Testing completed]: Verification on Tarako.
[Risk to taking this patch] (and alternatives if risky): Low risk.
[String changes made]: None.

Comment 11

[Carsten Book [:Tomcat]]

Hi,

sorry had to revert this change since i think this might have caused test failures like https://tbpl.mozilla.org/php/getParsedLog.php?id=36703800&tree=B2g-Inbound

Comment 12

[Marcus Cavanaugh [:mcav]]

It seems highly unlikely that this is the cause of those failures. Did backing it out solve the TBPL failures you saw, Carsten?

Comment 13

[Carsten Book [:Tomcat]]

yeah the failures cleared up after i backed this out, which might also be not really depend on this backout but on the other reverts and issues like bug 971585 we had. I guess if its unlikey that this one caused the test failure you could land this again :)

Comment 14

[Marcus Cavanaugh [:mcav]]

Thanks for the quick reply, Carsten. Since Travis was happy and this ought to have nothing to do with the failures you saw, I relanded. If you see further problems, of course, let me know.

master: 65c1f1385a00a897a62f35886bb03ec350b3b79c

Comment 15

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/7d716de0c186416b5b123baa1f3242e23d50529b
v1.3: https://github.com/mozilla-b2g/gaia/commit/8d159066289fdb4651e13e838fb91fb9226a2cd5

Comment 16

[Mike Lien[:mlien]]

verified and fixed with v1.3t
Gaia      98ce1340a6c27694f3b32a36b772e8da57caf19a
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/bdf9a55b4553
BuildID   20140409164001
Version   28.1