Closed Bug 1247122 Opened 4 years ago Closed 4 years ago

Intermittent 1228456.html,205735-1.xhtml | application crashed [@ mozilla::CycleCollectedJSRuntime::CycleCollectedJSRuntime(JSRuntime*, unsigned int, unsigned int)]

Categories

(Core :: DOM: Workers, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox45 --- wontfix
firefox46 --- affected
firefox47 --- fixed

People

(Reporter: KWierso, Assigned: baku)

References

Details

(Keywords: intermittent-failure)

Attachments

(1 file)

https://treeherder.mozilla.org/logviewer.html#?job_id=21345673&repo=mozilla-inbound
Summary: Intermittent 1228456.html | application crashed [@ mozilla::CycleCollectedJSRuntime::CycleCollectedJSRuntime(JSRuntime*, unsigned int, unsigned int)] → Intermittent 1228456.html,205735-1.xhtml | application crashed [@ mozilla::CycleCollectedJSRuntime::CycleCollectedJSRuntime(JSRuntime*, unsigned int, unsigned int)]
It looks like this is crashing on this MOZ_CRASH:

  mJSRuntime = JS_NewRuntime(aMaxBytes, aMaxNurseryBytes, aParentRuntime);
  if (!mJSRuntime) {
    MOZ_CRASH();
  }

I think that indicates that Firefox ran out of memory while trying to create a new JS runtime.
I'm moving this to DOM:Workers because it looks like the test creates 99 SharedWorkers, which seems like it could cause an OOM.
Component: JavaScript: GC → DOM: Workers
Yeah ... this seems like something likely to OOM.
Flags: needinfo?(amarchesini)
As noted in the summary, there are also a few (4 at current count) crashes in dom/xbl/crashtests/205735-1.xhtml. That test looks quite benign, but it is in fact the test that is run immediately after 1228456.html so presumably we are just running out of memory slightly later.
Version: 45 Branch → unspecified
Depends on: 1242774
Attached patch cycle.patchSplinter Review
This patch propagates the error in case we have allocation issues.
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8718920 - Flags: review?(khuey)
Comment on attachment 8718920 [details] [diff] [review]
cycle.patch

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

::: js/xpconnect/src/XPCJSRuntime.cpp
@@ +3539,2 @@
>  
>      if (self                                    &&

remove the null check for self while you're here.
Attachment #8718920 - Flags: review?(khuey) → review+
https://hg.mozilla.org/mozilla-central/rev/ac6fc75a900c
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
Comment on attachment 8718920 [details] [diff] [review]
cycle.patch

Approval Request Comment
[Feature/regressing bug #]: Cycle Collected JSRuntime initialization / workers
[User impact if declined]: Rarely a crash if the initialization of the worker JSRuntime fails.
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: the patch moves some code in order to return an error value in case the initialization of a new JS runtime fails.
[String/UUID change made/needed]: none
Attachment #8718920 - Flags: approval-mozilla-aurora?
This needs beta approval, not Aurora. (This is needed because the crash is apparently impacting e10s, as seen in bug 1259187).
Comment on attachment 8718920 [details] [diff] [review]
cycle.patch

See comment 15. This is already on Aurora, but not Beta. It is needed to reduce the OOM crash rate, which is unusually high with e10s.
Attachment #8718920 - Flags: approval-mozilla-aurora? → approval-mozilla-beta?
Comment on attachment 8718920 [details] [diff] [review]
cycle.patch

If this landed Tuesday or Wednesday this week in 46 beta 6, that would give us maybe a couple of days of e10s experiment information. I think we can wait for 47 here as it is not a high volume crash and seems to be often e10s related.
Attachment #8718920 - Flags: approval-mozilla-beta? → approval-mozilla-beta-
You need to log in before you can comment on or make changes to this bug.