Open
Bug 458484
Opened 16 years ago
Updated 15 years ago
mochitest-chrome, test_idcheck.xul : addressbook.xul leaks intermittently, due to having cards in outlook address book
Categories
(SeaMonkey :: MailNews: Address Book & Contacts, defect)
Tracking
(Not tracked)
NEW
People
(Reporter: sgautherie, Unassigned)
References
(Blocks 1 open bug, )
Details
(Keywords: memory-leak)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b1pre) Gecko/20081004 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
(After working around bug 391318.)
"chrome://messenger/content/addressbook/addressbook.xul"
leaks
{
TEST-PASS | runtests-leaks | WARNING leaked 10752 bytes during test execution (under threshold set at 19057 bytes)
TEST-PASS | runtests-leaks | leaked 1 instance of RDFServiceImpl with size 272 bytes
TEST-PASS | runtests-leaks | leaked 4 instances of nsAbOutlookCard with size 88 bytes each (352 bytes total)
TEST-PASS | runtests-leaks | leaked 1 instance of nsGenericFactory with size 16 bytes
TEST-PASS | runtests-leaks | leaked 4 instances of nsMapiEntry with size 8 bytes each (32 bytes total)
TEST-PASS | runtests-leaks | leaked 4 instances of nsRDFResource with size 24 bytes each (96 bytes total)
TEST-PASS | runtests-leaks | leaked 268 instances of nsStringBuffer with size 8 bytes each (2144 bytes total)
TEST-PASS | runtests-leaks | leaked 140 instances of nsVariant with size 56 bytes each (7840 bytes total)
}
Reporter | ||
Comment 1•16 years ago
|
||
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081116 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/8b3caddca8f5
+http://hg.mozilla.org/comm-central/rev/ebcb28433bfe)
This seems to be worse now:
nsTraceRefcntImpl::DumpStatistics: 919 entries
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 13368 bytes during test execution (threshold set at 8405 bytes)
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 1 instance of RDFServiceImpl with size 272 bytes
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 5 instances of nsAbOutlookCard with size 88 bytes each (440 bytes total)
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 1 instance of nsGenericFactory with size 16 bytes
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 5 instances of nsMapiEntry with size 8 bytes each (40 bytes total)
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 5 instances of nsRDFResource with size 24 bytes each (120 bytes total)
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 335 instances of nsStringBuffer with size 8 bytes each (2680 bytes total)
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 175 instances of nsVariant with size 56 bytes each (9800 bytes total)
Flags: wanted-seamonkey2?
Reporter | ||
Comment 2•16 years ago
|
||
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081120 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/48ec53300d76
+http://hg.mozilla.org/comm-central/rev/9a933d62a4ec)
Better, somehow, probably after bug 464831:
TEST-PASS | runtests-leaks | WARNING leaked 12960 bytes during test execution (threshold set at 21477 bytes)
TEST-PASS | runtests-leaks | leaked 5 instances of nsAbCardProperty with size 56 bytes each (280 bytes total)
TEST-PASS | runtests-leaks | leaked 325 instances of nsStringBuffer with size 8 bytes each (2600 bytes total)
TEST-PASS | runtests-leaks | leaked 180 instances of nsVariant with size 56 bytes each (10080 bytes total)
Target Milestone: --- → seamonkey2.0a2
Reporter | ||
Comment 3•16 years ago
|
||
(In reply to comment #2)
Looking for the triggering code line:
1) addressbook.xul
onload="OnLoadAddressBook()"
2) addressbook.js
2a) function OnLoadAddressBook()
setTimeout('OnLoadDirTree()', 0);
2b) function OnLoadDirTree()
SelectFirstAddressBook();
3) abCommon.js
3a) function SelectFirstAddressBook()
dirTree.view.selection.select(0);
Commenting out the latter line removes the leak...
Comment 4•16 years ago
|
||
I just checked on Mac and we're not seeing any of these leaks starting up and shutting down TB with the address book window. As the code is pretty much the same, and given the previous leak logs, this really points to a problem in the Outlook directory interfaces.
As to commenting out that last line, the only thing I can think is when we select the dirTree, its causing some datasource initialisation behind the scenes which is kicking the Outlook directory into loading its cards.
I just had a quick look through the outlook code:
In nsAbOutlookDirectory::GetChildCards, this line seems suspect:
236 nsCOMPtr<nsISupports> oldElement = mCardList.Get(&newKey) ;
Compare it with nsDirectoryService::Get:
635 nsCOMPtr<nsISupports> value = dont_AddRef(mHashtable.Get(&key));
It looks to me like we're overdoing our addrefs as a result.
I can't test this at the moment, and it may not be the leak your seeing, but probably worth investigating (oh and if it is, I wouldn't be surprised if you have 5 cards in your outlook address book).
Comment 5•16 years ago
|
||
(In reply to comment #4)
> I just had a quick look through the outlook code:
>
> In nsAbOutlookDirectory::GetChildCards, this line seems suspect:
>
> 236 nsCOMPtr<nsISupports> oldElement = mCardList.Get(&newKey) ;
I agree, Get() is already AddRef()ed and needs a dont_AddRef too.
Reporter | ||
Comment 6•16 years ago
|
||
(In reply to comment #4)
> [...] its causing some datasource initialisation behind the scenes
This is what I guessed; thanks for pinpointing the (exact) culprit code :-)
> It looks to me like we're overdoing our addrefs as a result.
I'll build a patch...
> [...] if you have 5 cards in your outlook address book).
I do ;->
Reporter | ||
Comment 7•16 years ago
|
||
Modified
236 nsCOMPtr<nsISupports> oldElement = dont_AddRef(mCardList.Get(&newKey));
doesn't help (or is not enough).
The leak is created when
225 NS_NewArrayEnumerator(aCards, cardList);
is executed...
Comment 8•16 years ago
|
||
(In reply to comment #7)
> The leak is created when
> 225 NS_NewArrayEnumerator(aCards, cardList);
> is executed...
I can't see how that leaks; if the caller leaked it would show up with other directories.
You could try adding "return retCode;" in various places to see how that affects the number of leaks. For instance, @227 I would expect no nsAbCardProperty leaks, but @237 I would expect 1 leak of nsAbCardProperty.
Reporter | ||
Comment 9•16 years ago
|
||
(In reply to comment #8)
> > 225 NS_NewArrayEnumerator(aCards, cardList);
> > is executed...
> I can't see how that leaks; if the caller leaked it would show up with other
> directories.
Don't ask me ;-/
> You could try adding "return retCode;" in various places [...]
That's exactly what I did: before 225, no leak; after 225, full leak :-|
Comment 10•16 years ago
|
||
So, I just noticed that we always clear the card list every time, which means we never find an existing card, so we can't leak them there...
Reporter | ||
Comment 11•16 years ago
|
||
It looks like bug 391318 comment 51 checkin "hided" this leak.
Mark, do you want to R.Fixed or keep it open to investigate further ?
Reporter | ||
Comment 12•16 years ago
|
||
(In reply to comment #11)
> It looks like bug 391318 comment 51 checkin "hided" this leak.
Confirmed, after commenting out that new code.
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081221 SeaMonkey/2.0a3pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/b839ff0630c6
+http://hg.mozilla.org/comm-central/rev/2a4c4c1f0feb + bug 469606 patch)
Comment 13•16 years ago
|
||
(In reply to comment #11)
> It looks like bug 391318 comment 51 checkin "hided" this leak.
>
> Mark, do you want to R.Fixed or keep it open to investigate further ?
I very much doubt it "hided" it, more likely fixed it. If we were leaking at the new enumerator, then its most likely that leaking the rdf service (which is what bug 391318 fixed) would have caused this to leak as a side-effect.
Therefore I don't think we can do anything more here.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → WORKSFORME
Reporter | ||
Comment 15•16 years ago
|
||
(In reply to comment #13)
> Therefore I don't think we can do anything more here.
Fwiw,
I still have a doubt, due to some kind of lack of explanation feeling;
I'd love something like bug 458486 comment 5.
Reporter | ||
Comment 16•16 years ago
|
||
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20081222 SeaMonkey/2.0a3pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/31dbaf4ca0c4
+http://hg.mozilla.org/comm-central/rev/113540504a46 + bug 469606 patch)
Er, actually, (comment 2) leak is still there:
only, it does not happen every time I run the --chrome test suite.
(That must have confused my yesterday tests :-<)
Reporter | ||
Comment 17•16 years ago
|
||
It looks like bug 472302 fixed this bug, but it caused bug 473686.
Then, let's wait for that later bug...
Status: REOPENED → NEW
Reporter | ||
Comment 18•16 years ago
|
||
(In reply to comment #16)
> it does not happen every time I run the --chrome test suite.
> (That must have confused my yesterday tests :-<)
(In reply to comment #17)
> It looks like bug 472302 fixed this bug, but it caused bug 473686.
> Then, let's wait for that later bug...
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090223 SeaMonkey/2.0b1pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/f6cdd2d6a9ea
+http://hg.mozilla.org/comm-central/rev/720d3a1ea63d)
Up to this build, this bug looked like to have been fixed.
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090228 SeaMonkey/2.0b1pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/f7f62131998d
+http://hg.mozilla.org/comm-central/rev/7ea34ef19dc4)
With this build, I reproduce this intermittent leak "again".
(Don't know if it was just haphazard for a while or if something "regressed" recently: anyway...)
Comment 19•16 years ago
|
||
Nice to have, but not a priority.
Flags: wanted-seamonkey2? → wanted-seamonkey2-
Reporter | ||
Comment 20•16 years ago
|
||
(In reply to comment #2)
Not seeing this very often for some time, but still there:
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090530 SeaMonkey/2.0b1pre] (experimental/_m-c_, home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/e44d9c0f4805
+http://hg.mozilla.org/comm-central/rev/62f2c362a9a4 + bug 493008 patches)
{
TEST-UNEXPECTED-FAIL | runtests-leaks | leaked 12960 bytes during test execution
}
Summary: test_idcheck.xul : addressbook.xul leaks (intermittently) → mochitest-chrome, test_idcheck.xul : addressbook.xul leaks intermittently, due to having cards in outlook address book
Reporter | ||
Comment 21•15 years ago
|
||
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1.1pre) Gecko/20090705 SeaMonkey/2.0b1pre] (comm-1.9.1-win32-unittest/1246823309) (W2Ksp4)
(http://hg.mozilla.org/releases/mozilla-1.9.1/rev/7b9cc253bcb8
+http://hg.mozilla.org/comm-central/rev/035fc7c4ed38)
(Bug still there.)
Updated•15 years ago
|
Target Milestone: seamonkey2.0a3 → ---
You need to log in
before you can comment on or make changes to this bug.
Description
•