Closed Bug 964386 Opened 11 years ago Closed 11 years ago

window.open()/.close() memory leak in content process

Categories

(Firefox OS Graveyard :: General, defect, P1)

ARM
Gonk (Firefox OS)
defect

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 961793
1.3 C3/1.4 S3(31jan)

People

(Reporter: m1, Assigned: huseby)

References

Details

(Keywords: memory-leak, perf, regression, Whiteboard: [c=memory p=2 s= u=1.3] [CR 606932],QARegressExclude)

Attachments

(5 files)

A memory leak is observed on v1.3 in content processes that repeatedly invoke window.open()/close() (eg. Communication app receiving MT calls repeatedly). The test app at bug 833077 attachment 704697 [details] can be used to easily reproduce after 10-20min.
Opps, test app is really at bug 833077 attachment 704662 [details]
blocking-b2g: --- → 1.3+
Whiteboard: [CR 606932]
Mike, Please help with next steps here as it blocks QC.
Flags: needinfo?(mlee)
Dave, Need you to take this issue and consider it your highest priority this sprint. Reach out to Ben Kelly, Kyle Huey, and/or Nicholas Nethercote as needed for help troubleshooting and resolving this. Thanks, Mike
Assignee: nobody → dhuseby
Status: NEW → ASSIGNED
Flags: needinfo?(mlee)
Keywords: mlk, perf
Whiteboard: [CR 606932] → [c=memory p= s= u=1.3] [CR 606932]
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
Any idea of a regression range?
(In reply to Dietrich Ayala (:dietrich) from comment #4) > Any idea of a regression range? If we can get clear STR with expected/actual results off that test app, then I can put someone on here to get a regression range.
(In reply to Jason Smith [:jsmith] from comment #5) > If we can get clear STR with expected/actual results off that test app, then > I can put someone on here to get a regression range. STR: 1. Run the app, hit "Start" 2. Run |adb shell watch b2g-procrank|. Watch the USS field for the app grow, within 10-15 min the memory usage is clearly abnormal. Keep going until it gets LMKed.
Component: Gaia::System::Window Mgmt → General
Attached file Test App (deleted) —
To get the window, do the following: 1. Push the test app to your device using the app manager ** https://developer.mozilla.org/en-US/Firefox_OS/Using_the_App_Manager 2. Follow the STR in comment 6 on a Buri 1.1 device & the latest 1.3 Buri device and identify the bug present here via comparing 1.1 vs. 1.3 behavior based on what comment 6 states 3. After you see the bug present here, work on getting the window here using the directions from comment 6
FWIW, I don't observe this issue at the tip of v1.2. Who is running the steps described in comment 8?
Flags: needinfo?(jsmith)
(In reply to Michael Vines [:m1] [:evilmachines] from comment #9) > FWIW, I don't observe this issue at the tip of v1.2. Who is running the > steps described in comment 8? Hello Michael, on 1.1 when running the Window test no windows actually open and close, but the completed count still rises along with the memory going up very slowly compared to 1.3. It's been going for about an hour now w/o LMKing. On 1.3 the app works properly (opening and closing windows), but I didn't get a LMK after 20min so I went to check 1.1. In short, what is the time range to LMK that I should be looking for when finding the regression window?
Not sure about 1.1, I only tried on 1.2 and 1.3. What does |adb shell b2g-procrank| output for you on 1.3 after running the app for 10-20min?
Is this reproducible on 1.3 after 20 mins? Please post logs here post 20 mins.
It is reproduceable in ~3minutes: Start: Every 2s: b2g-procrank 1970-01-01 20:59:22 APPLICATION PID Vss Rss Pss Uss cmdline b2g 326 84800K 72744K 58957K 54200K /system/b2g/b2g Homescreen 953 33996K 33992K 20800K 16344K /system/b2g/plugin-container WindowTest 1193 33956K 30944K 18317K 14380K /system/b2g/plugin-container (Preallocated a 1466 24240K 24236K 12777K 9448K /system/b2g/plugin-container Stop: Every 2s: b2g-procrank 1970-01-01 21:02:44 APPLICATION PID Vss Rss Pss Uss cmdline b2g 326 90696K 71824K 57952K 53188K /system/b2g/b2g WindowTest 1193 55952K 46124K 33406K 29440K /system/b2g/plugin-container Homescreen 953 33984K 33980K 20736K 16256K /system/b2g/plugin-container (Preallocated a 1466 24228K 24224K 12674K 9320K /system/b2g/plugin-container Note the 100% growth in WindowTest USS. On v1.2 this is not observed.
Flags: needinfo?(jsmith)
Attached image v1.4_WindowTest_Memory.png (deleted) —
I just ran a 15 minute test on master tip (v1.4) and there doesn't appear to be a memory leak there. See graph of memory usage over time.
Attached image v1.3_WindowTest_Memory.png (deleted) —
Here's a graph on v1.3 tip. The memory leak is definitely present.
I'm going to try out v1.3 from 1/1/14 and see if the memory leak is there. I'm hoping that will time box it to some recent changes that we can bisect.
I was able to repro this issue on the first build App Manager is able to install WindowTest, anything before the below build 10/24/2013 App Manager is not able to install WindowTest. .:First Broken Build:. Environmental Variables: Device: Buri 1.3 MOZ BuildID: 20131024154023 Gaia: 2feebdb1a2583928f32407d76d798f8654621e2b Gecko: 19fd3388c372 Version: 27.0a1 Firmware Version: v1.2-device.cfg
(In reply to gbennett from comment #17) > I was able to repro this issue on the first build App Manager is able to > install WindowTest, anything before the below build 10/24/2013 App Manager > is not able to install WindowTest. > > .:First Broken Build:. > Environmental Variables: > Device: Buri 1.3 MOZ > BuildID: 20131024154023 > Gaia: 2feebdb1a2583928f32407d76d798f8654621e2b > Gecko: 19fd3388c372 > Version: 27.0a1 > Firmware Version: v1.2-device.cfg Keep going farther back than this - there might have been a regression on the 10/24 Buri build where the app manager broke. I would imagine it wouldn't have been broken on every build before 10/24.
Since the bug is not found in master, it must have fixed in there between the v1.3 branch point and HEAD. I've been bisecting master to find when/where the memory leak was fixed. So far I've narrowed it down to being between 4e04586 and 678f815 on the master branch. I'll pretty soon which revision fixed the memory leak so we can figure out where the bug is.
I should point out that revision 4e04586 was on 1/23/14 and 678f815 was on 1/27/14. So I've narrowed it down to a small time window and about 100 revisions. I'm getting close.
Whiteboard: [c=memory p= s= u=1.3] [CR 606932] → [c=memory p= s= u=1.3] [CR 606932],QARegressExclude
I've got this bug down to a window of just 25 commits between 2457895 (bad) and 4544e52 (good) on the master branch. I was scanning the commits between those two points and found commit 0a07322 with the description: Bug 961802 -- Plugged leak in getUserMedia Denied code-path. Looking at the diff the change is in code related to windows. I'm testing that build right now to see if it fixes the memory leak.
(In reply to Dave Huseby [:huseby] from comment #21) > I've got this bug down to a window of just 25 commits between 2457895 (bad) > and 4544e52 (good) on the master branch. > > I was scanning the commits between those two points and found commit 0a07322 > with the description: Bug 961802 -- Plugged leak in getUserMedia Denied > code-path. > > Looking at the diff the change is in code related to windows. I'm testing > that build right now to see if it fixes the memory leak. I don't think that's going to be cause of this leak. That bug deals with gUM, which isn't even involved in window.open & window.close operations.
alrighty. well, I'm down to just 20 change sets, four more builds and I'll know which revision fixed it.
OS: Windows 7 → Gonk (Firefox OS)
Hardware: x86_64 → ARM
I'm checking my last revision. I'm down to two and will know very shortly which revision fixed the leak.
*drum roll*
So the winner is: d906a1c That's the revision that fixes the memory leak. I'm going through it now to see what changes would fix the memory leak.
(In reply to Dave Huseby [:huseby] from comment #26) > So the winner is: d906a1c > > That's the revision that fixes the memory leak. I'm going through it now to > see what changes would fix the memory leak. Gaia or Gecko? I'm trying to find the specific changeset this maps to hg or git.
It looks like this is Bug 961793
fwiw, this looks safe to uplift to 1.3
Depends on: 961793
Attached patch Bug_964386.patch (deleted) — Splinter Review
Here's the final patch from Bug 961793. This fixes the memory leak.
Attachment #8367821 - Flags: review?(fabrice)
Attached image v1.3_Patched_WindowTest_Memory.png (deleted) —
Here's memory consumption of the WindowTest app over a 20 minute test period. Notice that the memory leak is gone after patching.
(In reply to Dave Huseby [:huseby] from comment #30) > Created attachment 8367821 [details] [diff] [review] > Bug_964386.patch > > Here's the final patch from Bug 961793. This fixes the memory leak. Dave - I already marked the dependency a blocker here, so it will be automatically uplifted to 1.3.
Jason -- Awesome! Thanks.
Thanks!
Status: ASSIGNED → RESOLVED
blocking-b2g: 1.3+ → ---
Closed: 11 years ago
No longer depends on: 961793
Resolution: --- → DUPLICATE
Attachment #8367821 - Flags: review?(fabrice)
Whiteboard: [c=memory p= s= u=1.3] [CR 606932],QARegressExclude → [c=memory p=2 s= u=1.3] [CR 606932],QARegressExclude
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: