The default bug view has changed. See this FAQ.

Leaks detected by xpcshell tests failing on the atom leak assertion (IMAP)

NEW
Unassigned

Status

MailNews Core
Backend
--
critical
27 days ago
14 days ago

People

(Reporter: Jorg K (GMT+1), Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

27 days ago
+++ This bug was initially created as a clone of Bug #1337865 +++

PROCESS-CRASH | mailnews/imap/test/unit/test_imapFilterActions.js
PROCESS-CRASH | mailnews/imap/test/unit/test_imapFilterActionsPostplugin.js
PROCESS-CRASH | mailnews/db/gloda/test/unit/test_index_junk_imap_online.js
PROCESS-CRASH | mailnews/db/gloda/test/unit/test_index_junk_imap_offline.js

Let's look at more leaks in this new bug.
(Reporter)

Comment 1

27 days ago
Moving onto the next fire ;-)

A little bisection in test_imapFilterActionsPostplugin.js shows that two subtest cause the test to leak. When removing subtest MoveToFolder() and CopyToFolder() the leak is gone. Both of these when run as only test cause the leak.

Equally, when removing the MoveToFolder*() and CopyToFolder*() tests from test_imapFilterActions.js the test stops leaking. So by the looks of it, IMAP move or copy to folder by a filter causes the leak.
(Reporter)

Comment 2

27 days ago
Created attachment 8841360 [details] [diff] [review]
1342745-misc-fixes.patch - misc. fixes

Wherever you look, you find beautiful code, like here:
First dereference pointer, then check for != nullptr. Nice.
(Reporter)

Comment 3

27 days ago
Continuing leak search:

XPCOM_MEM_LOG_CLASSES=nsMsgDBFolder XPCOM_MEM_REFCNT_LOG=$HOME/refcounts.log mozilla/mach xpcshell-test mailnews/imap/test/unit/test_imapFilterActions.js

python mozilla/tools/rb/find_leakers.py $HOME/refcounts.log
000000000C2B2400 (12) @ <nsMsgDBFolder>
000000000A5C8C00 (13) @ <nsMsgDBFolder>

Repeat:
XPCOM_MEM_LOG_CLASSES=nsMsgDBFolder XPCOM_MEM_LOG_OBJECTS=12 XPCOM_MEM_REFCNT_LOG=$HOME/refcounts.log mozilla/mach xpcshell-test mailnews/imap/test/unit/test_imapFilterActions.js

python mozilla/tools/rb/find_leakers.py $HOME/refcounts.log
000000000C0C3000 (1) @ <nsMsgDBFolder>

perl -w mozilla/tools/rb/make-tree.pl --ignore-balanced --object 000000000C0C3000 < $HOME/refcounts.log > $HOME/xx.log

Sadly that died with "Out of memory during request for 22804 bytes, total sbrk() is 268267520 bytes!" after writing a file of 128 MB for further analysis. Aleth apparently found leaks using this tool, but I don't see how looking through an incomplete 128 MB file will reveal anything.
(Reporter)

Comment 4

25 days ago
Created attachment 8842019 [details] [diff] [review]
1342745-addref-release-nsImapMailFolder.patch - removing addref/release

More cleanup.
(Reporter)

Comment 5

25 days ago
Running the analysis tools on a reduced test_imapFilterActions.js again, this time for nsImapMailFolder I get:
python mozilla/tools/rb/find_leakers.py $HOME/refcounts2.log
000000000C0B2400 (2) @ <nsImapMailFolder>

Logging constructor and destructor, I see:
"=== nsImapMailFolder::nsImapMailFolder 000000000C2C2C00"
"=== nsImapMailFolder::nsImapMailFolder 000000000C380800" <== not destroyed
"=== nsImapMailFolder::nsImapMailFolder 000000000D9D1400"
"=== nsImapMailFolder::nsImapMailFolder 000000000D9D1800"
"=== nsImapMailFolder::~nsImapMailFolder 000000000D9D1800"
"=== nsImapMailFolder::nsImapMailFolder 000000000D9D1800"
"=== nsImapMailFolder::~nsImapMailFolder 000000000D9D1800"
"=== nsImapMailFolder::~nsImapMailFolder 000000000D9D1400"
"=== nsImapMailFolder::~nsImapMailFolder 000000000C2C2C00"

So the second one allocated doesn't get destroyed.

So further debugging shows that the second call is triggered from IMAPpump.js:96:
  imapAccount.incomingServer = IMAPPump.incomingServer;
and has this stack:
nsImapMailFolder::nsImapMailFolder() Line 212	C++
nsImapMailFolderConstructor(nsISupports * aOuter, const nsID & aIID, void * * aResult) Line 625	C++
RDFServiceImpl::GetResource(const nsACString_internal & aURI, nsIRDFResource * * aResource) Line 905	C++
nsImapMailFolder::AddSubfolderWithPath(nsAString_internal & name, ...) Line 371	C++
nsImapMailFolder::CreateClientSubfolderInfo(const nsACString_internal & folderName, ..) Line 941	C++
nsImapMailFolder::GetSubFolders(nsISimpleEnumerator * * aResult) Line 577	C++
nsMsgAccount::SetIncomingServer(nsIMsgIncomingServer * aIncomingServer) Line 157	C++

nsImapMailFolder.cpp around line 577 reads:
  GetFolderWithFlags(nsMsgFolderFlags::Inbox, getter_AddRefs(inboxFolder));
  if (!inboxFolder)
  {
    // create an inbox if we don't have one.
    CreateClientSubfolderInfo(NS_LITERAL_CSTRING("INBOX"), kOnlineHierarchySeparatorUnknown, 0, true); <=== 577
so it is the forcefully created inbox that leaks.

Not sure where it leaks, but I can't be far off now. Or maybe if doesn't leak and someone is just holding a reference to it.
(Reporter)

Comment 6

14 days ago
Test failures due to this were last seen Mon Mar 6, 2017, 11:16:18
https://treeherder.mozilla.org/#/jobs?repo=comm-central&revision=8be1ae77d2644e401fa6480a05ff234e70d0ccdf
The next push which covered these tests in debug mode of Tue Mar 7, 2017, 20:55:54, didn't show the failures.

However, when run locally, databases still leak.

I think what has happened is that some change in JS improved the GC. Let's see:
Last bad:   M-C: 966464a68a2cb3ca1125808e34abb5c1d3
First good: M-C: 3d341b9ba5353b6b8ab45b6ca03dcb1b2d
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=966464a68a2cb3ca1125808e34abb5c1d3&tochange=3d341b9ba5353b6b8ab45b6ca03dcb1b2d

I can't see it. Tooru-san, are you aware of any changes that could have magically fixed this?
Flags: needinfo?(arai.unmht)
Whiteboard: [Thunderbird-testfailure: X all debug]
nope
but I guess it's not about JS GC, but maybe CC.

then, if it's about atom, bug 1344848 might be related.
Flags: needinfo?(arai.unmht)
(Reporter)

Comment 8

14 days ago
Thanks, yes, bug 1344848.
You need to log in before you can comment on or make changes to this bug.