If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Firefox leaks when Yahoo Toolbar is installed

VERIFIED FIXED in mozilla1.9beta4

Status

()

Core
General
P2
normal
VERIFIED FIXED
10 years ago
8 years ago

People

(Reporter: Tomcat, Assigned: peterv)

Tracking

({mlk})

Trunk
mozilla1.9beta4
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(6 attachments)

(Reporter)

Description

10 years ago
Created attachment 298180 [details]
leak log

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012014 Firefox/3.0b3pre

Steps to reproduce:
New Firefox Profile
Install the Yahoo Toolbar https://addons.mozilla.org/en-US/firefox/addon/2032
Restart Firefox
Yahoo Toolbar loads - use the default settings, and click then on the "my yahoo" icon
Close Firefox after this site is completly loaded
Leak

0 TOTAL                                          31   559979  1147875    18506 ( 3255.57 +/-  3495.46)  2858985    25001 ( 2959.95 +/-  4484.71)





nsTraceRefcntImpl::DumpStatistics: 795 entries
nsDOMNodeAllocator leaked 238496 bytes 
nsStringStats
 => mAllocCount:          48282
 => mReallocCount:         6100
 => mFreeCount:           43979  --  LEAKED 4303 !!!
 => mShareCount:          44606
 => mAdoptCount:           5949
 => mAdoptFreeCount:       5947  --  LEAKED 2 !!!
Flags: blocking1.9?
Version: unspecified → Trunk
(Reporter)

Comment 1

10 years ago
Created attachment 298181 [details]
2nd leak log

also Firefox leaks in later session when this toolbar is installed:

nsDOMNodeAllocator leaked 238496 bytes 
nsStringStats
 => mAllocCount:          43179
 => mReallocCount:         4397
 => mFreeCount:           38903  --  LEAKED 4276 !!!
 => mShareCount:          40563
 => mAdoptCount:           5318
 => mAdoptFreeCount:       5316  --  LEAKED 2 !!!

  0 TOTAL                                          33   535707   867067    17847 ( 2753.52 +/-  3071.82)  2303435    24363 ( 2803.86 +/-  4180.62)
(Reporter)

Comment 2

10 years ago
Created attachment 298291 [details]
new leak log 

leak log using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b3pre) Gecko/2008012113 Minefield/3.0b3pre and with peter's fixes included
(Assignee)

Comment 3

10 years ago
Still looking at this, but the documents leak through a timer callback:

0x1899b218 [nsXPCWrappedJS (nsITimerCallback)] -> 0x17afb220 [JS Object (Object)] -> 0x17afb240 [JS Object (Function)] -> 0x17afb080 [JS Object (Call)] -> 0x16e76760 [JS Object (Object)] -> 0x16e71060 [JS Object (XPCWrappedNative_NoHelper)] -> 0x182db4a0 [XPCWrappedNative] -> 0x182db038 [nsXPCWrappedJS (nsIYahooDomBuilder)] -> 0x16e6b7c0 [JS Object (Object)] -> 0x16e6baa0 [JS Object (XMLDocument)] -> 0x182d9910 [XPCWrappedNative (XMLDocument)] -> 0x14d3e00 [nsDocument ([none]) about:blank]

0x189935c8 [nsXPCWrappedJS (nsITimerCallback)] -> 0x17afb160 [JS Object (Object)] -> 0x17afb180 [JS Object (Function)] -> 0x17afb080 [JS Object (Call)] -> 0x16e76760 [JS Object (Object)] -> 0x16e71060 [JS Object (XPCWrappedNative_NoHelper)] -> 0x182db4a0 [XPCWrappedNative] -> 0x182db038 [nsXPCWrappedJS (nsIYahooDomBuilder)] -> 0x16e6b7c0 [JS Object (Object)] -> 0x16e6baa0 [JS Object (XMLDocument)] -> 0x182d9910 [XPCWrappedNative (XMLDocument)] -> 0x14d3e00 [nsDocument ([none]) about:blank]
Assignee: nobody → peterv

Comment 4

10 years ago
> the documents leak through a timer callback

Due to bug 407201, perhaps?
(Assignee)

Comment 5

10 years ago
Though these look like repeating timers (TYPE_REPEATING_PRECISE).
(Assignee)

Comment 6

10 years ago
I can make the leaks drop from about 500k to about 1k by making the extension cancel its timers on xpcom-shutdown:

--- components/nsYahooFeedProcessor_old.js	2008-01-23 00:19:10.000000000 +0100
+++ components/nsYahooFeedProcessor.js	2008-01-23 00:21:23.000000000 +0100
@@ -106,6 +106,7 @@
 	init : function() {
 		if(this.loaded == false){
 			this.notifier = Components.classes["@mozilla.org/observer-service;1"].getService(Components.interfaces.nsIObserverService);
+			this.notifier.addObserver(this, "xpcom-shutdown", false);
 			this.uniconvert = Components.classes["@mozilla.org/intl/scriptableunicodeconverter"].createInstance(Components.interfaces.nsIScriptableUnicodeConverter);
 			this.uniconvert.charset = 'utf-8';
 			this.params = new YahooHashtable();
@@ -400,7 +401,20 @@
 		context = null;
 		this.loading = false;
 	},
-	/**
+	observe: function(subject, topic, data) {
+		if (topic == "xpcom-shutdown") {
+			if (this.timer) {
+				this.timer.cancel();
+				this.timer = null;
+			}
+			if (this.bm2timer) {
+				this.bm2timer.cancel();
+				this.bm2timer = null;
+			}
+			this.notifier.removeObserver(this, "xpcom-shutdown");
+		}
+	},
+ 	/**
 	* Cache the raw feed to file.
 	* The feed will only be cached if it's a guest feed.
 	*/

Can we stop and cancel all reoccurring timers before doing the final cycle collection? That should free the references to the timer callbacks.
(Assignee)

Comment 8

10 years ago
Created attachment 298697 [details] [diff] [review]
Patch v1

(In reply to comment #7)
> Can we stop and cancel all reoccurring timers before doing the final cycle
> collection?

Turns out we already do that, and just nulling out the timer on xpcom-shutdown is enough. But I can get the same effect with this patch, shutting down the TimerThread currently doesn't release the callbacks on shutdown, so if anything creates a cycle through the callback we'll leak the callback. In this case the component keeps the timer alive, and the callback keeps the component alive. Since the timer doesn't release its callback the cycle makes everything leak. Ideally we'd fix this with the cycle collector, but timers are marked threadsafe. The next best thing is probably manually releasing the callbacks on shutdown.

There's still a small number of objects leaking, I'll try to figure those out too.
Attachment #298697 - Attachment is patch: true
Attachment #298697 - Attachment mime type: application/octet-stream → text/plain
Attachment #298697 - Flags: superreview+
(Assignee)

Updated

10 years ago
Attachment #298697 - Flags: review?(jst)
Comment on attachment 298697 [details] [diff] [review]
Patch v1

Pav should look at this. Looks ok to me fwiw.
Attachment #298697 - Flags: review?(jst) → review?(pavlov)

Updated

10 years ago
Flags: blocking1.9? → blocking1.9+
Priority: -- → P1

Comment 10

10 years ago
Comment on attachment 298697 [details] [diff] [review]
Patch v1

should be OK
Attachment #298697 - Flags: review?(pavlov) → review+
(Reporter)

Comment 11

10 years ago
Created attachment 298970 [details]
leak log after patch v1

leak log from Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008012408 Firefox/3.0b3pre after the patch v1 was checked in

   0 TOTAL                                          30     1144  1174361       26 ( 3524.13 +/-  3688.99)  2822488       40 ( 2770.33 +/-  4332.76)
Lowering to P2 since the leak is much smaller now.
Priority: P1 → P2
(Assignee)

Comment 13

10 years ago
Created attachment 301165 [details] [diff] [review]
Patch 2 v1

I can fix the remaining leaks by making nsArray participate in cycle collection.
Comment on attachment 301165 [details] [diff] [review]
Patch 2 v1

This seems like a good idea in general.
Attachment #301165 - Flags: superreview+
Attachment #301165 - Flags: review+
(Assignee)

Comment 15

10 years ago
Thsi should be fixed now.
Status: NEW → RESOLVED
Last Resolved: 10 years ago
OS: Mac OS X → All
Hardware: PC → All
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta4
(Reporter)

Comment 16

10 years ago
verified fixed using Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US;
rv:1.9b4pre) Gecko/2008020717 Firefox/3.0b4pre not leaking anymore, great thanks peter.
Status: RESOLVED → VERIFIED

Updated

9 years ago
Blocks: 471296
Depends on: 512678
You need to log in before you can comment on or make changes to this bug.