Closed Bug 1480966 Opened Last year Closed Last year

Crash in class JS::SourceBufferHolder mozilla::dom::ScriptLoader::GetScriptSource

Categories

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

Unspecified
Windows 10
defect

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox-esr52 --- unaffected
firefox-esr60 --- unaffected
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed

People

(Reporter: calixte, Assigned: jonco)

References

(Blocks 1 open bug)

Details

(Keywords: crash, regression)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-4bf8bef0-5719-4450-8849-3164a0180803.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll class JS::SourceBufferHolder mozilla::dom::ScriptLoader::GetScriptSource dom/script/ScriptLoader.cpp:1912
1 xul.dll nsresult mozilla::dom::ScriptLoader::ProcessRequest dom/script/ScriptLoader.cpp:1997
2 xul.dll bool mozilla::dom::ScriptLoader::ProcessScriptElement dom/script/ScriptLoader.cpp:1318
3 xul.dll mozilla::dom::ScriptElement::MaybeProcessScript dom/script/ScriptElement.cpp:141
4 xul.dll void nsHtml5TreeOpExecutor::RunScript parser/html/nsHtml5TreeOpExecutor.cpp:738
5 xul.dll void nsHtml5TreeOpExecutor::RunFlushLoop parser/html/nsHtml5TreeOpExecutor.cpp:537
6 xul.dll static bool BackgroundFlushCallback parser/html/nsHtml5TreeOpExecutor.cpp:284
7 xul.dll std::_Func_impl_no_alloc<bool  
8 xul.dll mozilla::IdleTaskRunner::Run xpcom/threads/IdleTaskRunner.cpp:63
9 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1235

=============================================================

There are 3 crashes (from 3 installations) in nightly 63 starting with buildid 20180803104322. In analyzing the backtrace, the regression may have been introduced by patch [1] to fix bug 1475228.

[1] https://hg.mozilla.org/mozilla-central/rev?node=b1a05b0af027
Flags: needinfo?(jcoppeard)
Assignee: nobody → jcoppeard
Component: JavaScript Engine → DOM: Core & HTML
Flags: needinfo?(jcoppeard)
Priority: -- → P2
This is an OOM allocating a buffer to hold inline script data.

I'm surprised we are hitting this because I expect most of these to be small.

The patch handles this OOM as an error.  Alternatively we could crash with OOM here like we do for other memory allocations.  Baku, what do you think?
Attachment #8998145 - Flags: review?(amarchesini)
Comment on attachment 8998145 [details] [diff] [review]
bug1480966-get-source-oom

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

looks good to me.

::: dom/script/ScriptLoader.cpp
@@ +1912,5 @@
>      aRequest->Element()->GetScriptText(inlineData);
>  
>      size_t nbytes = inlineData.Length() * sizeof(char16_t);
>      JS::UniqueTwoByteChars chars(static_cast<char16_t*>(JS_malloc(aCx, nbytes)));
> +    if (!chars)

{}
Attachment #8998145 - Flags: review?(amarchesini) → review+
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/08d651dba0f6
Make ScriptLoader::GetScriptSource faillible on OOM r=baku
https://hg.mozilla.org/mozilla-central/rev/08d651dba0f6
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.