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



MailNews Core
2 months ago
2 months ago


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


Firefox Tracking Flags

(Not tracked)



(2 attachments)



2 months 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.

Comment 1

2 months 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.

Comment 2

2 months 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.

Comment 3

2 months 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>

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.

Comment 4

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

More cleanup.

Comment 5

2 months 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.

Comment 6

2 months ago
Test failures due to this were last seen Mon Mar 6, 2017, 11:16:18
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

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]
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)

Comment 8

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