856240 - Marketplace app sometimes dies due to OOM during identity login

Status: RESOLVED FIXED

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

Description

[krupa raj[:krupa]]

unagi build: 03/29 from b2g18 branch (v1.0)

Pre-conditions:
* Tester has their unagi phone set up for payments on -dev
* Tester has marketplace-dev app installed


steps to reproduce:
1. Launch marketplace-dev app
2. Start the purchase flow for any paid app
3. Sign in using Identity within the trustedUI
4. Enter PIN
5. Confirm your mobile number and enter the bango PIN
6. Notice that the Confirm your Purchase screen loads
7. Close the app and launch some other apps (I launched Settings and another app I had installed)
8. Return to the Marketplace app

expected behavior:
After step #8, user returns to the purchase flow within the trustedUI

actual behavior:
After step #8, user returns to the marketplace page where they initiated the payment from (details or search results page) and then the homepage loads.

reproducible? I was able to reproduce this behavior thrice.

Comment 1

[krupa raj[:krupa]]

Attachment: verbose logging

Comment 2

[Jason Smith [:jsmith]]

(In reply to krupa raj 82[:krupa] from comment #0)
> actual behavior:
> After step #8, user returns to the marketplace page where they initiated the
> payment from (details or search results page) and then the homepage loads.

This last step sounds like the marketplace app process *was* killed after you launched the other app - the user returns back to where the payment was from and then the homepage loads means you saw a screenshot of the old payment marketplace site and was returned to the homepage of the app since the app process was previously killed.

The strange issue is that we're getting killed a lot quicker than I initially expected. So my hunch is this likely a process manager bug in Justin's area of expertise. Likely a regression too, cause you mentioned, we weren't seeing this not too long ago. leo? nom for blocking consideration.

Comment 3

[Justin Lebar (not reading bugmail)]

Can you please include dmesg logs?  Unfortunately dmesg has some information that I'm not able to put into logcat.

Anyway Jason is probably right, it looks from the logs like something is getting killed here.  dmesg will give us more information.

Comment 4

[krupa raj[:krupa]]

Attachment: dmseg logging

Let me know if this works.

Comment 5

[Justin Lebar (not reading bugmail)]

Comment on attachment 732067 [details]
dmseg logging

> <4>[04-01 21:37:31.369] [28: kswapd0]select 693 (Marketplace Dev), adj 6, size 8392, to kill\
> <4>[04-01 21:37:31.369] [28: kswapd0]send sigkill to 693 (Marketplace Dev), adj 6, size 8392\

There you go; the marketplace app is in the background ("adj 6"), and we're killing it.

I think Gaia probably controls this.

Comment 6

[Jason Smith [:jsmith]]

Now this is starting to sound like another identity.js bug in Gaia::System.

Jed - Any ideas?

Comment 7

[Justin Lebar (not reading bugmail)]

Sounds like we just shouldn't setVisible(false) on the relevant frame.

Comment 8

[Justin Lebar (not reading bugmail)]

...until we have LRU logic in the process-priority manager (working on it).

Comment 9

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Jason Smith [:jsmith] from comment #6)
> Now this is starting to sound like another identity.js bug in Gaia::System.
> 
> Jed - Any ideas?

Hi, Jason: I think Justin has your answer: the marketplace app is in the background and so is more susceptible to being killed in an OOM situation.

(I had actually been wondering whether we would encounter precisely this OOM situation, with the os killing the process that spawned the process in the trusted ui.  I have been testing with this in mind, manually killing apps that call identity while the trusted ui is open, in order to ensure that things aren't left in some weird state if they crash or are crashed.)

I didn't know that LRU was in the works for the process-priority manager.  That's awesome news.

Comment 10

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Justin Lebar [:jlebar] from comment #5)
> I think Gaia probably controls this.

It does since bug 834871 landed. We remove the trusted UI associated with a killed app.

(In reply to krupa raj 82[:krupa] from comment #0)
> actual behavior:
> After step #8, user returns to the marketplace page where they initiated the
> payment from (details or search results page) and then the homepage loads.

That first page that you mention sounds like the app screenshot that is shown during the app launch.

Comment 11

[ewong]

linking bug to github project issues: https://github.com/mozilla/browserid/issues/3191

Comment 12

[Lucas Adamski [:ladamski]]

I think this needs to be resolved either way.  Depending on how long it takes for the LRU work to land, the setVisible interim fix may or may not be worth the churn (since I'm guessing we'll want to back it out once LRU lands).

Comment 13

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Lucas Adamski [:ladamski] (plz needinfo) from comment #12)
> I think this needs to be resolved either way.  Depending on how long it
> takes for the LRU work to land, the setVisible interim fix may or may not be
> worth the churn (since I'm guessing we'll want to back it out once LRU
> lands).

Trying setVisible as an interim fix sounds like a good plan to me.  I see you assigned it to me; I'll try to have a patch soon.

Comment 14

[(inactive after 6/18) Alive Kuo [:alive]]

Thanks Justin for c.c. me, after reading I think I could solve this but leave Jed to determine if he wants to keep this bug.

Comment 15

[Jed Parsons (use needinfo, please) [:jedp, :jparsons]]

(In reply to Alive Kuo [:alive] from comment #14)
> Thanks Justin for c.c. me, after reading I think I could solve this but
> leave Jed to determine if he wants to keep this bug.

Hi, Alive!  If you have a solution, please feel free to take the bug.

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Before the backend solution comes true, the only way I could figure out is maintaining a "I-CANNOT-BE-BROUT-TO-BACKGROUND" list of apps in somewhere of system and let window manager ignore setVisible(false) to that when it does app window transition(switch apps as bug description #0).

Yeah it'd be a dirty workaround.

Comment 17

[(inactive after 6/18) Alive Kuo [:alive]]

Justin, could you give me the bug # you mentioned in comment 8 so that I could add a comment about it? Thanks.

Comment 18

[(inactive after 6/18) Alive Kuo [:alive]]

After discussing w/ Tim, we are concerning to pollute Window Manager with such workaround may introduce some regression about visibility maintenance -- it now seems to be memory maintenance in gaia and this seems an edge case that, during trusted UI progress the user is supposed not to do app switch.

Before I start to do this workaround I'd like to know if this is necessary and really blocking.

Comment 19

[Justin Lebar (not reading bugmail)]

> Justin, could you give me the bug # you mentioned in comment 8 so that I could add a comment about 
> it?

bug 822325

We can probably have that in the leo timeframe, but personally I think you should probably do the workaround if it's relatively simple, and then we can back it out later.

Comment 20

[Wayne Chang [:wchang]]

Triage 4/10 - Leo+ this as it is related with payments, which users will be more concerned about if interrupted.

Comment 21

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Justin Lebar [:jlebar] from comment #19)
> > Justin, could you give me the bug # you mentioned in comment 8 so that I could add a comment about 
> > it?
> 
> bug 822325
> 
> We can probably have that in the leo timeframe, but personally I think you
> should probably do the workaround if it's relatively simple, and then we can
> back it out later.

I'm marking this as blocking bug 822325 and do the workaround in gaia at first, thanks.

Comment 22

[(inactive after 6/18) Alive Kuo [:alive]]

WIP
https://github.com/mozilla-b2g/gaia/pull/9083

Comment 23

[(inactive after 6/18) Alive Kuo [:alive]]

I am not able to test any identity function now :/

Comment 24

[(inactive after 6/18) Alive Kuo [:alive]]

(In reply to Alive Kuo [:alive] from comment #23)
> I am not able to test any identity function now :/

Identify works fine in v1.0.1(From marketplace)
But it breaks (trustedUI doesn't show) in v1-train and master.

Comment 25

[(inactive after 6/18) Alive Kuo [:alive]]

Attachment: https://github.com/mozilla-b2g/gaia/pull/9083

I'd verified the patch on v1.0.1 by:

Every 1.0s: adb shell b2g-ps --oom                                                     Thu Apr 11 15:07:36 2013

APPLICATION       OOM_ADJ  OOM_SCORE  OOM_SCORE_ADJ  USER     PID   PPID  VSIZE  RSS     WCHAN    PC         NA
ME
b2g                  0        259         0          root      109   1     214136 54116 ffffffff 400ee330 S /sy
stem/b2g/b2g
Marketplace          2        241         134        app_434   434   109   89132  20072 ffffffff 40022594 R /sy
stem/b2g/plugin-container
Browser              2        200         134        app_474   474   109   62340  12424 ffffffff 40108330 S /sy
stem/b2g/plugin-container
Browser              2        237         134        app_487   487   109   68280  19260 ffffffff 400b5330 S /sy
stem/b2g/plugin-container
Homescreen           4        386         267        app_517   517   109   67604  22304 ffffffff 40099330 S /sy
stem/b2g/plugin-container
(Preallocated a      2        215         134        app_567   567   109   57156  15204 ffffffff 40068330 S /sy
stem/b2g/plugin-container
Gallery              2        254         134        app_2400  2400  109   85236  22400 ffffffff 4009e330 S /sy
stem/b2g/plugin-container
(Preallocated a      6        472         400        root      2669  109   57156  19124 ffffffff 4008e330 S /sy
stem/b2g/plugin-container

Comment 26

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 736159 [details]
https://github.com/mozilla-b2g/gaia/pull/9083

Hum, I don't understand why am I missing this.

Comment 27

[(inactive after 6/18) Alive Kuo [:alive]]

master
https://github.com/mozilla-b2g/gaia/commit/dec958af6e40d213bd9b4a404110567a21222b03

Comment 28

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted commit dec958af6e40d213bd9b4a404110567a21222b03 as:
v1-train: b5e7a068c976d1b62a37005b4577483d3ba9e080

Comment 29

[Jason Smith [:jsmith]]

Krupa - On the 4/16 build tomorrow, can you check to see if this patch fixes the issue you hit?

Comment 30

[krupa raj[:krupa]]

(In reply to Jason Smith [:jsmith] from comment #29)
> Krupa - On the 4/16 build tomorrow, can you check to see if this patch fixes
> the issue you hit?

I will be on PTO from 04/16-04/17. I can check on Thursday.