976135 - crash in NS_ABORT_OOM(unsigned int) | nsString::nsString(nsString const&) from DOM storage calls spiked in Firefox 28.0b4

Status: RESOLVED INCOMPLETE

(Core :: DOM: Core & HTML, defect)

Description

[u279076]

This bug was filed from the Socorro interface and is 
report bp-0ade5a7e-c511-47b1-bddd-c733d2140224.
=============================================================
0 	xul.dll 	NS_ABORT_OOM(unsigned int) 	xpcom/base/nsDebugImpl.cpp
1 	xul.dll 	nsString::nsString(nsString const &) 	obj-firefox/dist/include/nsTString.h
2 	xul.dll 	nsStorage2SH::SetProperty(nsIXPConnectWrappedNative *,JSContext *,JSObject *,int,JS::Value *,bool *) 	dom/base/nsDOMClassInfo.cpp
3 	xul.dll 	XPC_WN_Helper_SetProperty(JSContext *,JS::Handle<JSObject *>,JS::Handle<int>,bool,JS::MutableHandle<JS::Value>) 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp
4 	mozjs.dll 	js::Shape::set(JSContext *,JS::Handle<JSObject *>,JS::Handle<JSObject *>,bool,JS::MutableHandle<JS::Value>) 	js/src/vm/Shape-inl.h
5 	mozjs.dll 	js::baseops::SetPropertyHelper<0>(JSContext *,JS::Handle<JSObject *>,JS::Handle<JSObject *>,JS::Handle<int>,unsigned int,JS::MutableHandle<JS::Value>,bool) 	js/src/jsobj.cpp
6 	mozjs.dll 	JSObject::setGeneric(JSContext *,JS::Handle<JSObject *>,JS::Handle<JSObject *>,JS::Handle<int>,JS::MutableHandle<JS::Value>,bool) 	js/src/jsobj.h
7 	mozjs.dll 	js::SetObjectElement(JSContext *,JS::Handle<JSObject *>,JS::Handle<JS::Value>,JS::Handle<JS::Value>,bool,JS::Handle<JSScript *>,unsigned char *) 	js/src/vm/Interpreter.cpp
8 	mozjs.dll 	js::jit::DoSetElemFallback 	js/src/jit/BaselineIC.cpp
9 		@0x435dec 	
10 		@0x431311 	
11 	mozjs.dll 	EnterBaseline 	js/src/jit/BaselineJIT.cpp
12 	mozjs.dll 	Interpret 	js/src/vm/Interpreter.cpp
13 	mozjs.dll 	js::Invoke(JSContext *,JS::Value const &,JS::Value const &,unsigned int,JS::Value *,JS::MutableHandle<JS::Value>) 	js/src/vm/Interpreter.cpp
14 	mozjs.dll 	JS_CallFunctionValue(JSContext *,JSObject *,JS::Value,unsigned int,JS::Value *,JS::Value *) 	js/src/jsapi.cpp
15 	xul.dll 	mozilla::dom::EventHandlerNonNull::Call(JSContext *,JS::Handle<JSObject *>,nsDOMEvent &,mozilla::ErrorResult &) 	obj-firefox/dom/bindings/EventHandlerBinding.cpp
16 	xul.dll 	mozilla::dom::EventHandlerNonNull::Call<nsISupports *>(nsISupports * const &,nsDOMEvent &,mozilla::ErrorResult &,mozilla::dom::CallbackObject::ExceptionHandling) 	obj-firefox/dist/include/mozilla/dom/EventHandlerBinding.h
17 	xul.dll 	nsJSEventListener::HandleEvent(nsIDOMEvent *) 	dom/src/events/nsJSEventListener.cpp
18 	ntdll.dll 	DaysAndFractionToTime 	
19 	ntdll.dll 	RtlTimeFieldsToTime 	
20 	KERNELBASE.dll 	SystemTimeToFileTime 	
21 	nss3.dll 	PR_Now 	nsprpub/pr/src/md/windows/ntmisc.c

More Reports:
https://crash-stats.mozilla.com/report/list?product=Firefox&signature=NS_ABORT_OOM%28unsigned+int%29+%7C+nsString%3A%3AnsString%28nsString+const%26%29
=============================================================

While this signature is not yet a topcrash (#64 in Beta), it appears to be spiking as of Firefox 28.0b4 (#3 in 3-day report): 
https://crash-analysis.mozilla.com/rkaiser/2014-02-23/2014-02-23.firefox.28.explosiveness.html

Top URLs:
52 	http://xavier3.jfc.com.ph/xavier04/site
6 	http://www.drudgereport.com/
6 	https://twitter.com/
4 	https://www.facebook.com/
3 	about:blank

Comment 1

[Benjamin Smedberg]

https://crash-stats.mozilla.com/search/?signature=%3DNS_ABORT_OOM%28unsigned+int%29+|+nsString%3A%3AnsString%28nsString+const%26%29&_facets=oom_allocation_size&_columns=date&_columns=signature&_columns=product&_columns=version&_columns=build_id&_columns=platform&_columns=oom_allocation_size

OOMAllocationSize appears to be content-controlled and can be up to multi-megabytes.

This is from DOM storage (other crashes from nsIDOMStorage_SetItem). This should probably use the infallible allocator, but I'm also wondering if there is a popular website which has started using domstorage and is causing a spike.

Comment 2

[u279076]

Flagging for qawanted to see if this can be reproduced on any of the top sites.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Unfortunately, there are all sorts of ways to trigger copies of strings whose length is content-controlled.  :(

The actual copy here happens in DOMStorage::SetItem, like so:

  108   nsresult rv = mCache->SetItem(this, aKey, nsString(aData), old);

I don't understand why DOMStorageCache::SetItem takes an nsString there instead of nsAString.... but even if it took the latter, we'd just end up copying a bit further along.  :(

Comment 4

[u279076]

This is no longer showing up at all in the explosiveness reports and sits at #87 overall in Firefox 28 so I'm not sure it's worth tracking this anymore. I'm not sure how to explain the spike.

Comment 5

[Benjamin Smedberg]

Yeah, I wouldn't track this based on that volume, and it's a "safe" crash no matter what.