Closed Bug 1490235 Opened 6 years ago Closed 6 years ago

Crash in OOM | large | NS_ABORT_OOM | mozilla::net::AltDataOutputStreamChild::Write

Categories

(Core :: Networking: Cache, defect, P2)

63 Branch
All
Windows
defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox-esr60 --- unaffected
firefox62 --- unaffected
firefox63 --- fixed
firefox64 --- fixed

People

(Reporter: philipp, Assigned: kershaw)

Details

(Keywords: crash, regression, Whiteboard: [necko-triaged])

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-94852c9e-8565-4f93-b744-8f6440180911.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll NS_ABORT_OOM xpcom/base/nsDebugImpl.cpp:628
1 xul.dll mozilla::net::AltDataOutputStreamChild::Write netwerk/protocol/http/AltDataOutputStreamChild.cpp:118
2 xul.dll void mozilla::dom::ScriptLoader::EncodeRequestBytecode dom/script/ScriptLoader.cpp:2596
3 xul.dll void mozilla::dom::ScriptLoader::EncodeBytecode dom/script/ScriptLoader.cpp:2547
4 xul.dll nsresult mozilla::detail::RunnableMethodImpl< xpcom/threads/nsThreadUtils.h:1219
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1161
6 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:519
7 xul.dll mozilla::ipc::MessagePump::Run ipc/glue/MessagePump.cpp:97
8 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:318
9 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:298

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

tab crashes with this signature started showing up in the 63.0a1 nightly cycle and are continuing in later versions as well.
Michal, any clue here?
Flags: needinfo?(michal.novotny)
I think at [1] we don't want to copy but use nsDependentCSubstring

Valentin, when you have time, can you fix and test this?

[1] https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/netwerk/protocol/http/AltDataOutputStreamChild.cpp#113
Assignee: nobody → valentin.gosu
Priority: -- → P2
Whiteboard: [necko-triaged]
Flags: needinfo?(michal.novotny)
Assignee: valentin.gosu → kershaw
Simply replace the nsCString with nsDependentCSubstring to avoid copying the data. In this way, we can also avoid the potential OOM crash.
Comment on attachment 9011531 [details]
Bug 1490235 - Use nsDependentCSubstring in AltDataOutputStreamChild::WriteDataInChunks

Honza Bambas (:mayhemer) has approved the revision.
Attachment #9011531 - Flags: review+
Pushed by kjang@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/49c829b004d8
Use nsDependentCSubstring in AltDataOutputStreamChild::WriteDataInChunks r=mayhemer
https://hg.mozilla.org/mozilla-central/rev/49c829b004d8
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Please request Beta approval on this when you're comfortable doing so.
Flags: needinfo?(kershaw)
Comment on attachment 9011531 [details]
Bug 1490235 - Use nsDependentCSubstring in AltDataOutputStreamChild::WriteDataInChunks

Approval Request Comment
[Feature/Bug causing the regression]:
Bug 1231565

[User impact if declined]:
OOM crash

[Is this code covered by automated tests?]:
Yes

[Has the fix been verified in Nightly?]:
Yes

[Needs manual test from QE? If yes, steps to reproduce]:
No. This is not easy to reproduce, since this is OOM crash.

[List of other uplifts needed for the feature/fix]:
No.

[Is the change risky?]:
No.

[Why is the change risky/not risky?]:
This patch simply avoid copying a huge string.

[String changes made/needed]:
No.
Flags: needinfo?(kershaw)
Attachment #9011531 - Flags: approval-mozilla-beta?
Comment on attachment 9011531 [details]
Bug 1490235 - Use nsDependentCSubstring in AltDataOutputStreamChild::WriteDataInChunks

Crash fix, uplift approved for 63 beta 10, thanks.
Attachment #9011531 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.