974070 - [build system] install-gaia fails to kill the app when there is a SEC column (peak)

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia, defect)

Description

[Julien Wajsberg [:julienw]]

I flashed a new gonk on my peak today, and now b2g-ps has a new column. Here is the output:

APPLICATION    SEC USER     PID   PPID  VSIZE  RSS     WCHAN    PC         NAME
b2g              0 root      127   1     189528 68748 ffffffff 400c05e0 S /system/b2g/b2g
(Nuwa)           0 root      330   127   52456  19452 ffffffff 400d55e0 S /system/b2g/plugin-container
Usage            2 app_344   344   330   64240  25484 ffffffff 400d55e0 S /system/b2g/plugin-container
Homescreen       2 app_420   420   330   75208  32936 ffffffff 400d55e0 S /system/b2g/plugin-container
Search Results   2 app_474   474   330   63844  23352 ffffffff 400d55e0 S /system/b2g/plugin-container
Messages         2 app_607   607   330   71076  31112 ffffffff 400d55e0 S /system/b2g/plugin-container
(Preallocated a  0 root      662   330   59616  20544 ffffffff 400d55e0 S /system/b2g/plugin-container


We see there is a "SEC" column which install-gaia doesn't know, and therefore pidMap gets keys like "Messages         2" which doesn't work.

Comment 1

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

b2g-ps used android toolbox ps[1] to get pid from system, I suggest parsing /proc directory to get app namd and pid directly.

[1] https://raw.github.com/android/platform_system_core/master/toolbox/ps.c

Comment 2

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to master

Hi Yuren,
thanks for your advice, but accessing the pid through /proc may be not so easy in javascript and async.

I modify part of the pid parser, so that we can accept dynamic column from now.

Please kindly check this patch.
Thanks.

Comment 3

[Julien Wajsberg [:julienw]]

Comment on attachment 8378114 [details] [review]
PR to master

This fixes the issue on my peak
(I haven't checked on another device, so please do)

Thanks for the quick fix!

Comment 4

[Julien Wajsberg [:julienw]]

Yuren, would you have some time to review this soon ? This is really slowing me a lot :)

Comment 5

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

Comment on attachment 8378114 [details] [review]
PR to master

this pr is okay but it is't reliable for all cases. I would like to give r+ it and George, could you file a follow up bug and use [gaia-build] on whiteboard field to track it?

Comment 6

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

this follow up bug is use /proc and /proc/PID/comm to get app name.

Comment 7

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

and seems we can't land it into master, please send a pull request to bubble-tea branch and merge it, but please don't close it until land to master.

Comment 8

[(Inactive after June) George Duan [:gduan] [:喬智]]

Attachment: PR to bubble-tea

PR to bubble-tea branch
https://groups.google.com/forum/#!topic/mozilla.dev.gaia/t-yUkIMcYsA

Comment 9

[(Inactive after June) George Duan [:gduan] [:喬智]]

Merge into bubble-tea
https://github.com/mozilla-b2g/gaia/commit/7084ef7df1852c74839180422b48c4c6de90c074

Comment 10

[(Inactive after June) George Duan [:gduan] [:喬智]]

In reply to comment 6,
https://bugzilla.mozilla.org/show_bug.cgi?id=975927 
use /proc and /proc/PID/comm to get app name

Comment 11

[Julien Wajsberg [:julienw]]

(In reply to Yuren [:yurenju][:小朱] from comment #7)
> and seems we can't land it into master, please send a pull request to
> bubble-tea branch and merge it, but please don't close it until land to
> master.

Why can't you?

This is a fix, not part of the build, so it's "safe" from the project's quality point of view.

Comment 12

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

makes sense, but new landing policy confuses me for some issue-fixing bug like this.

let's land this commit and keep an eye on bugzilla if any regressions.

Comment 13

[Julien Wajsberg [:julienw]]

what new policy?

(just kidding)

Comment 14

[Yuren [:yurenju] (AFK ~ Oct. 12) (volunteer mode)]

(shoulder shrug)

merged to master!
https://github.com/mozilla-b2g/gaia/commit/0e2d6f927bb55d6beacd5837bba916500255e37d

Comment 15

[(Inactive after June) George Duan [:gduan] [:喬智]]

Since it's already in master, close it.