Closed
Bug 740689
Opened 12 years ago
Closed 12 years ago
Context-menu module leaks memory, serious issue
Categories
(Add-on SDK Graveyard :: General, defect)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: mingyiliu, Assigned: ochameau)
Details
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/535.11 (KHTML, like Gecko) Chrome/17.0.963.79 Safari/535.11 Steps to reproduce: Created a simple demo addon using SDK 1.5 or 1.6rc1. The addon (attached to this report) requires only one module, context-menu, and does nothing in exports.main. Install the addon in FF13.0a2, then go to any page, like news.google.com. Check about:compartments?verbose, you'd see two compartments for the Google page. Then close the Google page, refresh the compartment page, and you'd see both compartments gone. Now open news.google.com again, and RIGHT click on the page once (or more if you want). Close the page, refresh the compartment page again. Actual results: None of the two compartments for news.google.com are gone. You could use about:memory?verbose and try to minimize the memory many times, it doesn't help. The two zoombie compartments will persist for a long time. Expected results: The two compartments for the closed page should've been gone once the page is closed. This should be a blocking bug.
Assignee | ||
Comment 1•12 years ago
|
||
Confirmed. Thanks for the report, it was really helpfull!! More details in the pull request for the fine details about what/how we were leaking.
Assignee: nobody → poirot.alex
Status: UNCONFIRMED → NEW
Ever confirmed: true
Attachment #611612 -
Flags: review?(rFobic)
Assignee | ||
Comment 2•12 years ago
|
||
The issue is quite important and the patch is quite small. I was wondering if we still have a chance to get it included in a last minute 1.6 RC? Any addon that simply require("context-menu") (even if they do not use this API, the simple fact of requiring it enable the leak), will leak the last webpage where they right clicked. But it shouldn't leak all documents where user right-clicked. So that it should not leak more than one document per browser window. I fixed one particular leak that was easy to reproduce, there might be others. I'm waiting for Mingyi confirmation that all leaks are gone.
Comment 3•12 years ago
|
||
(In reply to Alexandre Poirot (:ochameau) from comment #2) > The issue is quite important and the patch is quite small. > I was wondering if we still have a chance to get it included in a last > minute 1.6 RC? We're currently due to ship 1.6 tomorrow so if we think this is serious enough I think our options are to either do a new RC tomorrow instead and delay 1.6 for maybe a week, or do 1.6 as normal and do this as a 1.6.1 shortly after. > Any addon that simply require("context-menu") (even if they do not use this > API, the simple fact of requiring it enable the leak), will leak the last > webpage where they right clicked. But it shouldn't leak all documents where > user right-clicked. So that it should not leak more than one document per > browser window. Based on this summary, one document leaked per window, I don't think it is worth delaying the release, but let's talk about it in the weekly meeting.
Updated•12 years ago
|
Attachment #611612 -
Flags: review?(rFobic) → review+
Reporter | ||
Comment 4•12 years ago
|
||
Yes, it's fixed! Thanks for the quick work - I look forward to the final release.
Comment 5•12 years ago
|
||
Commit pushed to master at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/21181c0aca414078a410c6fe521944855912c303 Merge pull request #391 from ochameau/bug/740689-context-menu-leak Bug 740689: Fix context-menu leak of tabs where we opened a context menu r=@gozala
Assignee | ||
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 6•12 years ago
|
||
Commit pushed to stabilization at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/6ed75d9fc7b287590ebdf7bd5187e8959f8924a3 Merge pull request #391 from ochameau/bug/740689-context-menu-leak Bug 740689: Fix context-menu leak of tabs where we opened a context menu r=@gozala
(In reply to Dave Townsend (:Mossop) from comment #3) > (In reply to Alexandre Poirot (:ochameau) from comment #2) > > The issue is quite important and the patch is quite small. > > I was wondering if we still have a chance to get it included in a last > > minute 1.6 RC? > > We're currently due to ship 1.6 tomorrow so if we think this is serious > enough I think our options are to either do a new RC tomorrow instead and > delay 1.6 for maybe a week, or do 1.6 as normal and do this as a 1.6.1 > shortly after. > > > Any addon that simply require("context-menu") (even if they do not use this > > API, the simple fact of requiring it enable the leak), will leak the last > > webpage where they right clicked. But it shouldn't leak all documents where > > user right-clicked. So that it should not leak more than one document per > > browser window. > > Based on this summary, one document leaked per window, I don't think it is > worth delaying the release, but let's talk about it in the weekly meeting. So, this landed on the master development branch. It didn't make it to stabilization branch in time for the 1.6 release, but we're going to be pushing out a 1.6.1 hotfix for this sometime in the next week.
Comment 8•12 years ago
|
||
Commit pushed to release at https://github.com/mozilla/addon-sdk https://github.com/mozilla/addon-sdk/commit/6ed75d9fc7b287590ebdf7bd5187e8959f8924a3 Merge pull request #391 from ochameau/bug/740689-context-menu-leak
You need to log in
before you can comment on or make changes to this bug.
Description
•