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)

x86
Windows 2000
defect
Not set
normal

Tracking

(Not tracked)

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) }
Blocks: 458486
Depends on: 458988
Blocks: 452205
[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?
[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
(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...
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).
(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.
(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 ;->
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...
(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.
(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 :-|
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...
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 ?
Depends on: 391318
Flags: wanted-seamonkey2?
Target Milestone: seamonkey2.0a2 → seamonkey2.0a3
(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)
(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
V.WorksForMe then.
Status: RESOLVED → VERIFIED
No longer blocks: 452205
(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.
[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 :-<)
Blocks: 452205
Status: VERIFIED → REOPENED
No longer depends on: 391318
Flags: wanted-seamonkey2?
Resolution: WORKSFORME → ---
Summary: test_idcheck.xul : addressbook.xul leaks → test_idcheck.xul : addressbook.xul leaks (intermittently)
It looks like bug 472302 fixed this bug, but it caused bug 473686. Then, let's wait for that later bug...
Status: REOPENED → NEW
No longer blocks: 452205
(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...)
Nice to have, but not a priority.
Flags: wanted-seamonkey2? → wanted-seamonkey2-
(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
[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.)
Target Milestone: seamonkey2.0a3 → ---
You need to log in before you can comment on or make changes to this bug.