979718 - nsComponentManagerImpl::mFactories should be nsClassHashtable not nsDataHashtable

Status: RESOLVED WONTFIX

(Core :: XPCOM, defect)

Description

[Andrew McCreight [:mccr8]]

I saw some stacks like this:
19:47:23     INFO -  Direct leak of 256 byte(s) in 8 object(s) allocated from:
19:47:23     INFO -      #0 0x446395 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
19:47:23     INFO -      #1 0x7f1d94635b18 in moz_xmalloc /builds/slave/try-l64-asan-00000000000000000/build/memory/mozalloc/mozalloc.cpp:52
19:47:23     INFO -      #2 0x7f1d89ba05be in nsComponentManagerImpl::RegisterModule(mozilla::Module const*, mozilla::FileLocation*) /builds/slave/try-l64-asan-00000000000000000/build/xpcom/components/nsComponentManager.cpp:411
19:47:23     INFO -      #3 0x7f1d89ba1f93 in nsComponentManagerImpl::ManifestBinaryComponent(nsComponentManagerImpl::ManifestProcessingContext&, int, char* const*) /builds/slave/try-l64-asan-00000000000000000/build/xpcom/components/nsComponentManager.cpp:582
19:47:23     INFO -      #4 0x7f1d89b93f84 in ParseManifest(NSLocationType, mozilla::FileLocation&, char*, bool) /builds/slave/try-l64-asan-00000000000000000/build/xpcom/components/ManifestParser.cpp:647

Inlining limits how useful the stack is, but reading the code I think the problem is that the hash table entries are allocated but never freed.

Comment 1

[Andrew McCreight [:mccr8]]

Presumably the same thing for mContractIDs.

Comment 2

[Andrew McCreight [:mccr8]]

The leak is small, and we're probably going to hold this stuff until shutdown anyways (?) so it doesn't seem like a huge deal, except for establishing LSAN cleanliness.

Comment 3

[Benjamin Smedberg]

We leak the nsFactoryEntries on purpose at the moment because we store the pointer to them in both mFactories and mContractIDs. See http://mxr.mozilla.org/mozilla-central/source/xpcom/components/nsComponentManager.cpp#785 for where we could free them if we cared, but I really don't think we should care: these are process-liftime objects.

Comment 4

[Andrew McCreight [:mccr8]]

Alright.  I guess we can ignore all leaks from these nsComponentManagerImpl functions that allocate factories.

Comment 5

[Andrew McCreight [:mccr8]]

Well, I may have to ignore all leaks from nsComponentManagerImpl, because I saw various stuff entrained by factories getting leaked, too.  We'll see.

Comment 6

[Andrew McCreight [:mccr8]]

Attachment: clear factories to reduce LSAN log spamming, in lieu of whitelisting.

I added code to delete the factories at the place bsmedberg pointed out.

Comment 7

[Andrew McCreight [:mccr8]]

That fixed the factory leaks, though this line is still leaking stuff (reftest):
        mModule = mLoader->LoadModule(mFile);

I guess that's probably just another intentional leak?

Direct leak of 2816 byte(s) in 32 object(s) allocated from:
    #0 0x446395 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7f10c20b0b18 in moz_xmalloc /somepath/memory/mozalloc/mozalloc.cpp:52
    #2 0x7f10b6052d1b in Load /somepath/xpcom/components/nsComponentManager.cpp:741
    #3 0x7f10b6052d1b in nsFactoryEntry::GetFactory() /somepath/xpcom/components/nsComponentManager.cpp:1783
    #4 0x7f10b60537b9 in nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) /somepath/xpcom/components/nsComponentManager.cpp:1084

Direct leak of 264 byte(s) in 3 object(s) allocated from:
    #0 0x446395 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7f10c20b0b18 in moz_xmalloc /somepath/memory/mozalloc/mozalloc.cpp:52
    #2 0x7f10b6052d1b in Load /somepath/xpcom/components/nsComponentManager.cpp:741
    #3 0x7f10b6052d1b in nsFactoryEntry::GetFactory() /somepath/xpcom/components/nsComponentManager.cpp:1783
    #4 0x7f10b6053611 in nsComponentManagerImpl::CreateInstance(nsID const&, nsISupports*, nsID const&, void**) /somepath/xpcom/components/nsComponentManager.cpp:999

Direct leak of 264 byte(s) in 3 object(s) allocated from:
    #0 0x446395 in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:74
    #1 0x7f10c20b0b18 in moz_xmalloc /somepath/memory/mozalloc/mozalloc.cpp:52
    #2 0x7f10b6052d1b in Load /somepath/xpcom/components/nsComponentManager.cpp:741
    #3 0x7f10b6052d1b in nsFactoryEntry::GetFactory() /somepath/xpcom/components/nsComponentManager.cpp:1783
    #4 0x7f10b604a28c in operator void ** /somepath/xpcom/components/nsComponentManager.cpp:999
    #5 0x7f10b604a28c in nsComponentManagerImpl::GetService(nsID const&, nsID const&, void**) /somepath/xpcom/components/nsComponentManager.cpp:1251
    #6 0x7f10b947fe97 in nsJSCID::GetService(JS::Handle<JS::Value>, JSContext*, unsigned char, JS::MutableHandle<JS::Value>) /somepath/js/xpconnect/src/XPCJSID.cpp:748

Comment 8

[Benjamin Smedberg]

That stack doesn't make much sense:  mModule = mLoader->LoadModule(mFile) doesn't call moz_xmalloc directly, although the LoadModule call itself might.

The two normal implementations of LoadModule are mozJSComponentLoader::LoadModule and nsNativeModuleLoader::LoadModule.

Comment 9

[Andrew McCreight [:mccr8]]

These are opt builds, so the stacks can be weird.  I'm not sure how it would be inlining a virtual method call like that, though...

Comment 10

[(no longer active)]

(In reply to comment #9)
> These are opt builds, so the stacks can be weird.  I'm not sure how it would be
> inlining a virtual method call like that, though...

Virtual methods can be inlined just find if the compiler can somehow determine the concrete type of the pointer (haven't looked to see if that is the case here though.)