Closed Bug 1431646 Opened 3 years ago Closed 2 years ago

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

Categories

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

58 Branch
x86_64
macOS
defect

Tracking

()

RESOLVED FIXED
mozilla60
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- wontfix
firefox59 --- wontfix
firefox60 --- fixed

People

(Reporter: philipp, Assigned: baku)

References

Details

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

Crash Data

Attachments

(1 file)

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]
Attached 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: 2 years ago
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.