1396594 - Stylo: Crash in core::result::unwrap_failed<T> | hashglobe::hash_map::HashMap<T>::entry<T>

Status: RESOLVED WORKSFORME

(Core :: CSS Parsing and Computation, defect, P3)

Description

[Calixte Denizet (:calixte)]

This bug was filed from the Socorro interface and is 
report bp-d54a2975-9c55-4567-a92f-c755c0170903.
=============================================================

There are 14 crashes in nightly 57 starting with buildid 20170902220453. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1393656.

[1] https://hg.mozilla.org/mozilla-central/rev?node=c54db40f50d7b28208b79fdc6907bcc220f9c13d

Comment 1

[Bobby Holley (:bholley)]

Looks like these are allocation crashes, which we can now fix in bug 1395064?

Comment 2

[Manish Goregaokar [:manishearth]]

Wait, what?

How did these not cause OOM crashes before?

Also, this is on amd64/x68-64? Aren't OOMs harder to trigger there, especially because of overcommit?

Then again, if we're writing to an allocation immediately I guess overcommit is never an issue; just actually running out of memory, which still feels rare.

Perhaps we should include data about the capacity being allocated in the panic message. Thoughts?

Comment 3

[Bobby Holley (:bholley)]

(In reply to Manish Goregaokar [:manishearth] from comment #2)
> Wait, what?
> 
> How did these not cause OOM crashes before?

It's possible that it was just ending up with a different signature.

> 
> Also, this is on amd64/x68-64? Aren't OOMs harder to trigger there,
> especially because of overcommit?

You can still run out of physical memory. "System Memory Use Percentage" on that report is at 98.

> 
> Then again, if we're writing to an allocation immediately I guess overcommit
> is never an issue; just actually running out of memory, which still feels
> rare.
> 
> Perhaps we should include data about the capacity being allocated in the
> panic message. Thoughts?

The regular crash reporter knows how to annotate crash reports with the OOM size:

http://searchfox.org/mozilla-central/rev/2aa0806c598ec433e431728f5ddd3a6847c1a417/toolkit/crashreporter/nsExceptionHandler.cpp#1827

We should probably call an FFI function to do set gOOMAllocationSize before we panic.

Comment 4

[Josh Matthews [:jdm]]

I'll do the crash report hookup in bug 1396899.

Comment 5

[Bobby Holley (:bholley)]

(In reply to Josh Matthews [:jdm] from comment #4)
> I'll do the crash report hookup in bug 1396899.

Great, thanks!

Comment 6

[Bobby Holley (:bholley)]

Marking this as P3, since this should be fixed by a bug that's already P1. We should verify that the signature is fixed once bug 1395064 lands.

Comment 7

[Bobby Holley (:bholley)]

I don't see any crashes for this signature anymore.