Closed
Bug 1396290
Opened 7 years ago
Closed 7 years ago
Crash in OOM | large | NS_ABORT_OOM | nsACString::Assign
Categories
(Core :: DOM: File, defect)
Tracking
()
RESOLVED
FIXED
mozilla57
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox55 | --- | wontfix |
firefox56 | --- | fixed |
firefox57 | --- | fixed |
People
(Reporter: philipp, Assigned: baku)
References
Details
(Keywords: crash, regression)
Crash Data
Attachments
(1 file)
3.32 KB,
patch
|
froydnj
:
review+
gchang
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
This bug was filed from the Socorro interface and is report bp-24f886a0-c84f-4435-a74a-3cc5a0170902. ============================================================= Crashing Thread (27), Name: IPDL Background Frame Module Signature Source 0 xul.dll NS_ABORT_OOM(unsigned int) xpcom/base/nsDebugImpl.cpp:610 1 xul.dll nsACString::Assign(nsACString const&) xpcom/string/nsTSubstring.cpp:421 2 xul.dll nsStringInputStream::Clone(nsIInputStream**) xpcom/io/nsStringStream.cpp:387 3 xul.dll mozilla::BasePrincipal::GetOriginSuffix(nsACString&) caps/BasePrincipal.cpp:300 4 xul.dll nsMultiplexInputStream::Clone(nsIInputStream**) xpcom/io/nsMultiplexInputStream.cpp:1034 5 xul.dll nsMultiplexInputStream::Clone(nsIInputStream**) xpcom/io/nsMultiplexInputStream.cpp:1034 6 xul.dll NS_CloneInputStream(nsIInputStream*, nsIInputStream**, nsIInputStream**) xpcom/io/nsStreamUtils.cpp:923 7 xul.dll mozilla::dom::IPCBlobInputStreamStorage::GetStream(nsID const&, nsIInputStream**) dom/file/ipc/IPCBlobInputStreamStorage.cpp:151 8 xul.dll mozilla::dom::IPCBlobInputStreamParent::HasValidStream() dom/file/ipc/IPCBlobInputStreamParent.cpp:164 9 xul.dll mozilla::ipc::BackgroundParentImpl::RecvPIPCBlobInputStreamConstructor(mozilla::ipc::PIPCBlobInputStreamParent*, nsID const&, unsigned __int64 const&) ipc/glue/BackgroundParentImpl.cpp:336 10 xul.dll mozilla::ipc::PBackgroundParent::OnMessageReceived(IPC::Message const&) obj-firefox/ipc/ipdl/PBackgroundParent.cpp:2272 11 xul.dll mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) ipc/glue/MessageChannel.cpp:2092 12 xul.dll mozilla::ipc::MessageChannel::DispatchMessageW(IPC::Message&&) ipc/glue/MessageChannel.cpp:2018 13 xul.dll mozilla::ipc::MessageChannel::RunMessage(mozilla::ipc::MessageChannel::MessageTask&) ipc/glue/MessageChannel.cpp:1887 14 xul.dll mozilla::ipc::MessageChannel::MessageTask::Run() ipc/glue/MessageChannel.cpp:1920 15 xul.dll nsThread::ProcessNextEvent(bool, bool*) xpcom/threads/nsThread.cpp:1446 16 xul.dll NS_ProcessNextEvent(nsIThread*, bool) xpcom/threads/nsThreadUtils.cpp:480 17 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*) ipc/glue/MessagePump.cpp:369 18 xul.dll MessageLoop::RunHandler() ipc/chromium/src/base/message_loop.cc:319 19 xul.dll MessageLoop::Run() ipc/chromium/src/base/message_loop.cc:299 20 xul.dll nsThread::ThreadFunc(void*) xpcom/threads/nsThread.cpp:506 21 nss3.dll _PR_NativeRunThread nsprpub/pr/src/threads/combined/pruthr.c:397 22 nss3.dll pr_root nsprpub/pr/src/md/windows/w95thred.c:95 23 ucrtbase.dll thread_start<unsigned int (__stdcall*)(void*)> 24 kernel32.dll BaseThreadInitThunk 25 mozglue.dll patched_BaseThreadInitThunk mozglue/build/WindowsDllBlocklist.cpp:815 26 ntdll.dll __RtlUserThreadStart 27 ntdll.dll _RtlUserThreadStart these OOM crashes are starting with firefox 55 (related to bug 1340710?) on 32bit versions on windows but the reports get more frequent during the 56.0b cycle. last week this has been the #20 top crash on beta accounting for 0.6% of browser crashes.
Comment 1•7 years ago
|
||
(In reply to [:philipp] from comment #0) > these OOM crashes are starting with firefox 55 (related to bug 1340710?) on > 32bit versions on windows but the reports get more frequent during the 56.0b > cycle. last week this has been the #20 top crash on beta accounting for 0.6% > of browser crashes. Unlikely. If anything, those changes probably make OOM less likely. And the origin strings in question are small enough that they're unlikely to show up in a significant number of OOM crashes. In any case, the GetOriginSuffix() call in that stack is a red herring. The crash actually happens when copying the nsStringInputStream contents. This is more likely related to bug 1353629.
Comment 2•7 years ago
|
||
baku, does anything stand out here? This is not a huge top crash in beta, but it does seem to have gotten worse in 56 beta.
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 3•7 years ago
|
||
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8904872 -
Flags: review?(nfroyd)
Comment 4•7 years ago
|
||
Comment on attachment 8904872 [details] [diff] [review] string.patch Review of attachment 8904872 [details] [diff] [review]: ----------------------------------------------------------------- This is great, will probably help fix other crashes, too. I don't know where to put the comment below; deleting it altogether is an option. WDYT? ::: xpcom/io/nsStringStream.cpp @@ +385,5 @@ > { > + RefPtr<nsStringInputStream> ref = new nsStringInputStream(); > + > + // Use Assign() here because we don't want the life of the clone to be > + // dependent on the life of the original stream. This comment seems a little out of place here. Maybe it should cite the `Assign` call in SetData instead, or maybe SetData should have an explicit comment about `Assign`?
Attachment #8904872 -
Flags: review?(nfroyd) → review+
Pushed by amarchesini@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/6dada8a78fd1 nsStringStream should propagate the OOM error of SetData() when cloned, r=froydnj
Comment 6•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/6dada8a78fd1
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Comment 7•7 years ago
|
||
This grafts cleanly to Beta. Please nominate it for approval when you're comfortable doing so.
Flags: needinfo?(amarchesini)
Assignee | ||
Comment 8•7 years ago
|
||
Comment on attachment 8904872 [details] [diff] [review] string.patch Approval Request Comment [Feature/Bug causing the regression]: nsStringInputStream [User impact if declined]: OOM not correctly handled. [Is this code covered by automated tests?]: no [Has the fix been verified in Nightly?]: hard to reproduce. [Needs manual test from QE? If yes, steps to reproduce]: no [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: low. [Why is the change risky/not risky?]: It's just an error propagation of the return value of string::Assign() [String changes made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8904872 -
Flags: approval-mozilla-beta?
Comment 9•7 years ago
|
||
Comment on attachment 8904872 [details] [diff] [review] string.patch fix an OOM. Beta56+.
Attachment #8904872 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 10•7 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/65dc1a82b406
You need to log in
before you can comment on or make changes to this bug.
Description
•