Closed Bug 1281168 Opened 3 years ago Closed 3 years ago

Crash in OOM | unknown | js::AutoEnterOOMUnsafeRegion::crash | js::ObjectGroupCompartment::NewEntry::Lookup::Lookup

Categories

(Core :: JavaScript Engine, defect, critical)

48 Branch
x86
Windows 7
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla51
Tracking Status
firefox48 --- fixed
firefox49 + fixed
firefox50 --- fixed
firefox51 --- fixed

People

(Reporter: marcia, Assigned: jonco)

Details

(Keywords: crash, regression, topcrash)

Crash Data

Attachments

(2 files)

[Tracking Requested - why for this release]: Seems like a new crash in beta that we will probably want to keep an eye on. 

This bug was filed from the Socorro interface and is 
report bp-6224b84a-12bc-4de1-9e9b-46c9f2160614.
=============================================================

Seen while looking at Beta crashes. Link to crashes: http://bit.ly/28Ktni4. Looks like right now #19 top crash on beta.

Frame 	Module 	Signature 	Source
0 	xul.dll 	js::AutoEnterOOMUnsafeRegion::crash(char const*) 	js/src/jscntxt.cpp:1236
1 	xul.dll 	js::ObjectGroupCompartment::NewEntry::Lookup::Lookup(js::Class const*, js::TaggedProto, JSObject*) 	js/src/vm/ObjectGroup.cpp:388
2 	xul.dll 	js::ObjectGroup::defaultNewGroup(js::ExclusiveContext*, js::Class const*, js::TaggedProto, JSObject*) 	js/src/vm/ObjectGroup.cpp:493
3 	xul.dll 	js::NewObjectWithGivenTaggedProto(js::ExclusiveContext*, js::Class const*, JS::Handle<js::TaggedProto>, js::gc::AllocKind, js::NewObjectKind, unsigned int) 	js/src/jsobj.cpp:729
4 	xul.dll 	js::NewObjectWithGivenTaggedProto(js::ExclusiveContext*, js::Class const*, JS::Handle<js::TaggedProto>, js::NewObjectKind, unsigned int) 	js/src/jsobjinlines.h:636
5 	xul.dll 	XPCWrappedNative::Init(XPCNativeScriptableCreateInfo const*) 	js/xpconnect/src/XPCWrappedNative.cpp:792
6 	xul.dll 	js::NativeObject::getChildProperty(js::ExclusiveContext*, JS::Handle<js::NativeObject*>, JS::Handle<js::Shape*>, JS::MutableHandle<js::StackShape>) 	js/src/vm/Shape.cpp:451
7 	xul.dll 	XPCWrappedNative::InitTearOff(XPCWrappedNativeTearOff*, XPCNativeInterface*, bool) 	js/xpconnect/src/XPCWrappedNative.cpp:1197
8 	xul.dll 	XPCCallContext::~XPCCallContext() 	js/xpconnect/src/XPCCallContext.cpp:231
9 	xul.dll 	XPCConvert::NativeData2JS(JS::MutableHandle<JS::Value>, void const*, nsXPTType const&, nsID const*, nsresult*) 	js/xpconnect/src/XPCConvert.cpp:344
10 	xul.dll 	XPC_WN_GetterSetter(JSContext*, unsigned int, JS::Value*) 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1163
11 	xul.dll 	js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) 	js/src/vm/Interpreter.cpp:480
Nothing in that stack instantiates AutoEnterOOMUnsafeRegion or calls oomUnsafe.crash. The stack above the crash appears to be an empty constructor. All crashes are under windows, so maybe a miscompilation of some sort?

Next step is to load the minidump up and see if we can divine what's actually happening here.
Track this as the number of this crash in 48 is high.
Hi Terrence, 
Is there any updates or new findings here?
Flags: needinfo?(terrence)
In Beta 5 this currently sits as the #17 top crash.
Sorry for the delay getting to this: there have been a large number of more easily actionable release blockers in my queue this cycle.

Looking at the most recent 10 reports, they are all appear virtual address space exhaustion on 32bit. Therefore, this is a normal OOM|small, albeit with a heavily mangled stack. In this case, moving to a 64bit build of Firefox on windows should solve 90% of the crashes I looked at in this pile.

As with other OOM|small, it is not directly actionable and should be addressed in an ongoing manner by the memshrink project.
Flags: needinfo?(terrence)
This appears to be the same basic issue that we have a resolution for in bug 1260785 so I'm going to dup it there.

This is another instance where 80-90% of the crash reports have <250MiB of available virtual memory, but plenty of physical memory to spare. We really, really need to ship a 64bit firefox on windows.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1260785
Removing tracking flags for 48/49/50 since we will track in the duplicate bug.
(In reply to Terrence Cole [:terrence] from comment #6)
> This appears to be the same basic issue that we have a resolution for in bug
> 1260785 so I'm going to dup it there.
> 
> This is another instance where 80-90% of the crash reports have <250MiB of
> available virtual memory, but plenty of physical memory to spare. We really,
> really need to ship a 64bit firefox on windows.
> 
> *** This bug has been marked as a duplicate of bug 1260785 ***

Hello Terrence: We still are getting these crashes in RC2 (currently #10) - is that expected if the other bug has been fixed?
Flags: needinfo?(terrence)
(In reply to Marcia Knous [:marcia - use ni] from comment #8)
> (In reply to Terrence Cole [:terrence] from comment #6)
> > This appears to be the same basic issue that we have a resolution for in bug
> > 1260785 so I'm going to dup it there.
> > 
> > This is another instance where 80-90% of the crash reports have <250MiB of
> > available virtual memory, but plenty of physical memory to spare. We really,
> > really need to ship a 64bit firefox on windows.
> > 
> > *** This bug has been marked as a duplicate of bug 1260785 ***
> 
> Hello Terrence: We still are getting these crashes in RC2 (currently #10) -
> is that expected if the other bug has been fixed?

It's quite possible I misattributed this. The stacks are not particularly helpful. Or it might just be that this code is not fallible so making the underlying hash fallible didn't actually do anything useful.
Status: RESOLVED → REOPENED
Flags: needinfo?(terrence)
Resolution: DUPLICATE → ---
Assignee: nobody → jcoppeard
Today this sits as the #8 top crash on the latest RC (20160726073904), so it moved up a few slots since I posted the other day.
Use fallible hashing for ObjectGroup::NewEntry.
Attachment #8776593 - Flags: review?(terrence)
Comment on attachment 8776593 [details] [diff] [review]
bug1281168-new-entry-oom

Review of attachment 8776593 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!

::: js/src/vm/TaggedProto.cpp
@@ +49,5 @@
> +bool
> +js::TaggedProto::hasUniqueId() const
> +{
> +    if (!isObject())
> +        return true;

It's odd that "does not need a UID" is synonymous with "has a UID". Oh well.
Attachment #8776593 - Flags: review?(terrence) → review+
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/15a9785381a4
Make hashing ObjectGroupCompartment::NewEntry fallible r=terrence
https://hg.mozilla.org/mozilla-central/rev/15a9785381a4
Status: REOPENED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla51
[Tracking Requested - why for this release]: We should track this for 49 - this is currently the #6 top crash on 48 beta.
Jon, could you request the uplift to aurora and beta as it seems to impact these two branches? Thanks
Flags: needinfo?(jcoppeard)
Comment on attachment 8776593 [details] [diff] [review]
bug1281168-new-entry-oom

Approval Request Comment
[Feature/regressing bug #]: Bug 1224038.
[User impact if declined]: Possible crash on OOM.
[Describe test coverage new/current, TreeHerder]: On m-c since 2nd August.
[Risks and why]: Low.
[String/UUID change made/needed]: None.
Flags: needinfo?(jcoppeard)
Attachment #8776593 - Flags: approval-mozilla-beta?
Attachment #8776593 - Flags: approval-mozilla-aurora?
Comment on attachment 8776593 [details] [diff] [review]
bug1281168-new-entry-oom

Fix a top crash, taking it to aurora & beta
Attachment #8776593 - Flags: approval-mozilla-beta?
Attachment #8776593 - Flags: approval-mozilla-beta+
Attachment #8776593 - Flags: approval-mozilla-aurora?
Attachment #8776593 - Flags: approval-mozilla-aurora+
Approval Request Comment
[Feature/regressing bug #]: Bug 1224038.
[User impact if declined]: Possible crash on OOM.
[Describe test coverage new/current, TreeHerder]: On m-c since 2nd August.
[Risks and why]: Low.
[String/UUID change made/needed]: None.
Attachment #8780095 - Flags: review+
Attachment #8780095 - Flags: approval-mozilla-release?
Comment on attachment 8780095 [details] [diff] [review]
bug1281168-release

this is a new crash with 48, top crash #3, taking it.
Attachment #8780095 - Flags: approval-mozilla-release? → approval-mozilla-release+
You need to log in before you can comment on or make changes to this bug.