Crash in _platform_memmove$VARIANT$Haswell | <name omitted> | nsPipeInputStream::ReadSegments

RESOLVED FIXED in Firefox 60

Status

()

defect
P2
critical
RESOLVED FIXED
2 years ago
Last year

People

(Reporter: philipp, Assigned: baku)

Tracking

({crash, regression})

58 Branch
mozilla60
x86_64
macOS
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox57 unaffected, firefox58 wontfix, firefox59 wontfix, firefox60 fixed)

Details

(Whiteboard: [necko-triaged], crash signature)

Attachments

(1 attachment)

This bug was filed from the Socorro interface and is
report bp-8ec3df5a-cc07-4c78-bdaa-32e920180119.
=============================================================

Top 10 frames of crashing thread:

0 libsystem_platform.dylib _platform_memmove$VARIANT$Haswell 
1 XUL <name omitted> xpcom/io/nsStreamUtils.cpp:827
2 XUL nsPipeInputStream::ReadSegments xpcom/io/nsPipe3.cpp:1448
3 XUL  netwerk/base/nsNetUtil.cpp:1538
4 XUL mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:243
5 XUL nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:228
6 XUL non-virtual thunk to nsThreadPool::Run xpcom/threads/nsThreadPool.cpp:156
7 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1040
8 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:517
9 XUL mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:364

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

these browser crashes on macos are regressing in firefox 58 - the patch from bug 1417448 already tried to fix them.
a small fraction of them come with MOZ_RELEASE_ASSERT(!mReadState.mActiveRead) from bug 1133939.
ni? baku for the regressing bug.
Flags: needinfo?(amarchesini)
P2 -> we know the regressing bug so it should be easily actionable.
Assignee: nobody → amarchesini
Priority: -- → P2
Whiteboard: [necko-triaged]
Posted patch buffer.patchSplinter Review
This is again a speculative fix because I don't see why we have a runnable dispatched, when there is not a call to NS_ReadInputStreamToBuffer() in any thread.

It seems that the operation is already completed, somehow, but we still have ::Run() execution. Here a boolean to avoid this.
Flags: needinfo?(amarchesini)
Attachment #8954305 - Flags: review?(bugs)
Comment on attachment 8954305 [details] [diff] [review]
buffer.patch

this is bizarre. make the bool atomic.
Attachment #8954305 - Flags: review?(bugs) → review+
Is the stack correct at all here?  It seems unrelated to the assertion in the crash report:

MOZ_RELEASE_ASSERT(!mReadState.mActiveRead)

That assertion suggests there are multiple threads trying to read from the same nsPipeInputStream at the same time.
Flags: needinfo?(amarchesini)
FWIW, I asked the same question and baku said that assertion was only on one crash report.
> MOZ_RELEASE_ASSERT(!mReadState.mActiveRead)

I don't see this assertion in any of the crash report I have checked so far. They are always related to NS_CopySegmentToBuffer().
Flags: needinfo?(amarchesini)
Hmm, ok.

Is it safe to do this on a potentially non-flat string?

https://searchfox.org/mozilla-central/rev/056a4057575029f58eb69ae89bc7b6c3c11e2e28/netwerk/base/nsNetUtil.cpp#1891

Although I don't see any places that pass non-flat strings in for the buffer.
Maybe some diagnostic assertions around mBuffer, mWrittenData, and mCount might be able to help isolate the problem.
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/33cf111a7265
Adding a completed boolean to know if the operation is completed in NS_ReadInputStreamToBuffer(), r=smaug
https://hg.mozilla.org/mozilla-central/rev/33cf111a7265
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla60
marking 59 wontfix, though there are no crashes since 59.0b11
You need to log in before you can comment on or make changes to this bug.