Closed Bug 731184 Opened 12 years ago Closed 9 days ago

OOM Crash [@ InMemoryDataSource::~InMemoryDataSource] due to unhandled alloc failure of InMemoryDataSource::Init()

Categories

(Core Graveyard :: RDF, defect)

x86_64
Linux
defect
Not set
critical

Tracking

(Not tracked)

RESOLVED INCOMPLETE

People

(Reporter: decoder, Unassigned)

References

Details

(Keywords: crash)

Crash Data

Tested on m-c revision 66e4d53697c2: The method InMemoryDataSource::Init() may fail to allocate but this doesn't seem to be handled properly somewhere as allocation failure will cause the following crash:


Program received signal SIGSEGV, Segmentation fault.
0x00002aaaacfb7e05 in InMemoryDataSource::~InMemoryDataSource (this=0xdc8530, __in_chrg=<optimized out>) at /srv/repos/browser/mozilla-central/rdf/base/src/nsInMemoryDataSource.cpp:956
956         PR_LOG(gLog, PR_LOG_NOTICE,
#0  0x00002aaaacfb7e05 in InMemoryDataSource::~InMemoryDataSource (this=0xdc8530, __in_chrg=<optimized out>) at /srv/repos/browser/mozilla-central/rdf/base/src/nsInMemoryDataSource.cpp:956
#1  0x00002aaaacfb7e50 in InMemoryDataSource::~InMemoryDataSource (this=0xdc8530, __in_chrg=<optimized out>) at /srv/repos/browser/mozilla-central/rdf/base/src/nsInMemoryDataSource.cpp:959
#2  0x00002aaaacfb80b0 in InMemoryDataSource::Internal::Release (this=0xdc8620) at /srv/repos/browser/mozilla-central/rdf/base/src/nsInMemoryDataSource.cpp:991
#3  0x00002aaaacfb8de5 in NS_NewRDFInMemoryDataSource (aOuter=<optimized out>, aIID=..., aResult=0x7fffffffade0) at /srv/repos/browser/mozilla-central/rdf/base/src/nsInMemoryDataSource.cpp:880
#4  0x00002aaaad282bea in CreateInstanceByContractID (aResult=0x7fffffffade0, aIID=..., aDelegate=0x0, aContractID=0x2aaaada36ca4 "@mozilla.org/rdf/datasource;1?name=in-memory-datasource", this=<optimized out>) at /srv/repos/browser/mozilla-central/xpcom/components/nsComponentManager.cpp:1064
#5  nsComponentManagerImpl::CreateInstanceByContractID (this=<optimized out>, aContractID=<optimized out>, aDelegate=0x0, aIID=..., aResult=0x7fffffffade0) at /srv/repos/browser/mozilla-central/xpcom/components/nsComponentManager.cpp:1013
#6  0x00002aaaad244e35 in nsCreateInstanceByContractID::operator() (this=0x7fffffffad48, aIID=<optimized out>, aInstancePtr=0x7fffffffade0) at /srv/repos/browser/mozilla-central/objdir-ff-gcc64dbg/xpcom/build/nsComponentManagerUtils.cpp:210
#7  0x00002aaaacfd07e3 in assign_from_helper (aIID=..., helper=..., this=0xdc6398) at ../../../dist/include/nsCOMPtr.h:1232


The backtrace of the failing allocation is as follows:

#0 /srv/repos/browser/mozilla-central/objdir-ff-gcc64dbg/dist/bin/libmozalloc.so(moz_malloc+0x5f) [0x2aaaaab2415c] (aab2415c)
#1 PL_DHashTableInit at objdir-ff-gcc64dbg/xpcom/build/pldhash.cpp:270
#2 InMemoryDataSource::Init() at rdf/base/src/nsInMemoryDataSource.cpp:912
#3 NS_NewRDFInMemoryDataSource(nsISupports*, nsID const&, void**) at rdf/base/src/nsInMemoryDataSource.cpp:874
#4 nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) at xpcom/components/nsComponentManager.cpp:1065
#5 nsCreateInstanceByContractID::operator()(nsID const&, void**) const at objdir-ff-gcc64dbg/xpcom/build/nsComponentManagerUtils.cpp:211
#6 nsCOMPtr<nsIRDFDataSource>::assign_from_helper(nsCOMPtr_helper const&, nsID const&) at objdir-ff-gcc64dbg/dist/include/nsCOMPtr.h:1232
#7 nsWindowDataSourceConstructor at xpfe/components/windowds/nsWindowDataSource.cpp:576
#8 nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) at xpcom/components/nsComponentManager.cpp:1065
#9 nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) at xpcom/components/nsComponentManager.cpp:1466
I don't understand what allocation is failing incorrectly, can you help explain?

Both of the calls to PL_DHashTableInit in InMemoryDataSource::Init are properly checked and if they fail we clear out .ops.

Both of the calls in the destructor check .ops before doing anything.

But the stack given for the crash site also doesn't appear to be using the allocation you mention: it appears to be on the PR_LOG line. Is it possible that the line "gLog = PR_NewLogModule("InMemoryDataSource");" is failing? If so, we use that pattern unchecked throughout the codebasse and I don't think we should change it.
I double checked this, and the allocation trace (the second trace of comment 0) is the trace emitted for the allocation failure. My patch emits this trace when it artificially causes an OOM for a certain call to a fallible allocator (e.g. moz_malloc). However, I checked in GDB what caused the crash, and it's indeed gLog being NULL:

(gdb) p gLog
$5 = (PRLogModuleInfo *) 0x0

Maybe this isn't visible from the allocation trace because this frame is optimized away.

My problem with accepting oom crashes that don't use the intended abort functions to signal a controlled crash will cause automated tools like mine to detect these as errors.

I would have proposed to use something like | if (UNLIKELY(!gLog)) someAbort() | but this is NSPR code anyway so I guess it cannot be changed that easily.
Yes, we cannot change NSPR. We could wrap NSPR with an infallible version of PR_NewLogModule. But since most of our log modules are allocated during startup or statically i.e. static PRLogModuleInfo *gBMPLog = PR_NewLogModule("BMPDecoder"); We decided in the past that it really doesn't matter. Can you skip PR_NewLogModule in your OOM checker?
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #3)
> Can you skip PR_NewLogModule in your OOM checker?

Not using this function name, because it doesn't appear anywhere in the trace. But I could probably check if the failing source line is PR_LOG, or exclude ~InMemoryDataSource if that's always the function triggering the crash. I think I'll check how this behaves on other Mochitests (this is just a single run).
Product: Core → Core Graveyard
Status: NEW → RESOLVED
Closed: 9 days ago
Resolution: --- → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.