Closed Bug 1417448 Opened 2 years ago Closed 2 years ago

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

Categories

(Core :: Networking, defect, critical)

59 Branch
Unspecified
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla59
Tracking Status
firefox-esr52 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed
firefox59 --- fixed

People

(Reporter: sphilp, Assigned: baku)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is
report bp-cee067a7-78e8-4628-aa10-75fca0171115.
=============================================================

Top 10 frames of crashing thread:

0 libsystem_platform.dylib _platform_memmove$VARIANT$Haswell 
1 XUL &lt;name omitted&gt; xpcom/io/nsStreamUtils.cpp:825
2 XUL nsPipeInputStream::ReadSegments xpcom/io/nsPipe3.cpp:1448
3 XUL  netwerk/base/nsNetUtil.cpp:1584
4 XUL mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:246
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:1037
8 XUL NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:513
9 XUL mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:334

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

Details: Nightly 59.0a1(2017-11-14) on OSX 10.13.1. This crash was on wake from sleep, I hadn't interacted with the browser just lifted my mbp's lid. Prior to putting my laptop to sleep, I had a video playing that was likely still downloading playback segments... Also had several other tabs open like gdocs, gmail, etc. Sleep was overnight, probably 10 hours or so.
Dragana might know this more
Flags: needinfo?(dd.mozilla)
This looks related to bug 1415081.
The first crash in on 10th. Nov. Bug 1415081 landed on 10th. Nov. The are old crashes on Thunderbird, but they have a bit different crash stack and a different crash address.

baku, can you take a look? (I am not sure if you have only moved code around or not)

Thanks
Blocks: 1415081
Flags: needinfo?(dd.mozilla) → needinfo?(amarchesini)
Attached patch check.patchSplinter Review
It's hard to understand what happens here. Checking the crash reports I can see that there are no threads using NS_ReadInputStreamTo{String,Buffer}. This means that probably the operation is already completed.
But if the operation is completed, why do we have a runnable dispatched?
Could it be that lock.Wait() fails?

I added the following checks:

1. lock.Wait() return value check.
2. BufferWriter::Run() does everything protected by lock
3. StealBuffer also needs to be protected, plus it set mBufferSize to 0.
Assignee: nobody → amarchesini
Flags: needinfo?(amarchesini)
Attachment #8930054 - Flags: review?(bugs)
Based on code, Wait() doesn't ever fail, at least doesn't return any error value.
Comment on attachment 8930054 [details] [diff] [review]
check.patch

So this is a guess fix?
Attachment #8930054 - Flags: review?(bugs) → review+
Yes it is.
Pushed by amarchesini@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2173819ac195
Better use of monitors in NS_ReadInputStreamToBuffer, r=smaug
https://hg.mozilla.org/mozilla-central/rev/2173819ac195
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Please request Beta approval on this when you get a chance.
Flags: needinfo?(amarchesini)
Comment on attachment 8930054 [details] [diff] [review]
check.patch

Approval Request Comment
[Feature/Bug causing the regression]: bug 1415081
[User impact if declined]: a crash can occur.
[Is this code covered by automated tests?]: no, race condition
[Has the fix been verified in Nightly?]: yes
[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?]: no
[Why is the change risky/not risky?]: the buffer access is protected by mutex and its correctly nullified when needed.
[String changes made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8930054 - Flags: approval-mozilla-beta?
Comment on attachment 8930054 [details] [diff] [review]
check.patch

Fix a crash. Beta58+.
Attachment #8930054 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Looks as if we are still hitting this crash on Mac nightly after the fix went in on 11-20: http://bit.ly/2ANRCOY
should we reopen the bug or file a new one for the remaining crashes?
Flags: needinfo?(amarchesini)
file a new bug, thanks.
Flags: needinfo?(amarchesini)
See Also: → 1431646
You need to log in before you can comment on or make changes to this bug.