2004304 - Release assert crash in [@ mozilla::ipc::data_pipe_detail::DataPipeBase::ProcessSegmentsInternal]

Status: RESOLVED DUPLICATE of bug 2005113

(Core :: IPC, defect)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/927b1938-9fe0-4080-939e-a181b0251204

MOZ_CRASH Reason:

MOZ_RELEASE_ASSERT(!link->mProcessingSegment) (Only one thread may be processing a segment at a time)

Top 10 frames:

0  libxul.so  MOZ_CrashSequence(void*, long)  mfbt/Assertions.h:242
0  libxul.so  mozilla::ipc::data_pipe_detail::DataPipeBase::ProcessSegmentsInternal(unsigne...  ipc/glue/DataPipe.cpp:317
1  libxul.so  mozilla::ipc::DataPipeReceiver::ReadSegments(nsresult (*)(nsIInputStream*, vo...  ipc/glue/DataPipe.cpp:644
1  libxul.so  mozilla::ipc::DataPipeReceiver::Read(char*, unsigned int, unsigned int*)  ipc/glue/DataPipe.cpp:633
2  libxul.so  mozilla::dom::InputStreamHolder::Read(char*, unsigned int, unsigned int*)  dom/streams/UnderlyingSourceCallbackHelpers.h:197
2  libxul.so  mozilla::dom::InputToReadableStreamAlgorithms::PullFromInputStream(JSContext*...  dom/streams/UnderlyingSourceCallbackHelpers.cpp:467
3  libxul.so  mozilla::dom::InputToReadableStreamAlgorithms::OnInputStreamReady(nsIAsyncInp...  dom/streams/UnderlyingSourceCallbackHelpers.cpp:313
4  libxul.so  mozilla::dom::InputStreamHolder::OnInputStreamReady(nsIAsyncInputStream*)  dom/streams/UnderlyingSourceCallbackHelpers.cpp:229
5  libxul.so  mozilla::ipc::DataPipeReceiver::AsyncWait(nsIInputStreamCallback*, unsigned i...  ipc/glue/DataPipe.cpp:671
5  libxul.so  NS_NewCancelableRunnableFunction<mozilla::ipc::DataPipeReceiver::AsyncWait(ns...  xpcom/threads/nsThreadUtils.h:637

This is not a new issue but it flew under the radar for a long time before spiking on the release channel, but only in Fenix.

Comment 1

[Andrew McCreight [:mccr8]]

Nika, any idea what might be going wrong here? Thanks.

Comment 2

[BugBot [:suhaib / :marco]]

The bug is linked to a topcrash signature, which matches the following criteria:

• Top 10 AArch64 and ARM crashes on beta
• Top 10 AArch64 and ARM crashes on release

For more information, please visit BugBot documentation.

Comment 3

[Tyson Smith [:tsmith]]

Bug 2005113 is what I see trying to reproduce this issue. There is a Pernosco session attached.

Comment 4

[Matthew Gaudet (he/him) [:mgaudet]]

Ted, just flagging this because it seems the majority of crashes on this signature are Fenix 145.0.2.

Comment 5

[Nika Layzell [:nika] (ni? for response)]

(In reply to Andrew McCreight [:mccr8] from comment #1)

Nika, any idea what might be going wrong here? Thanks.

This is quite strange. This flag is only set during the region where ProcessSegmentsInternal is on the stack for that specific nsIInputStream. It should be impossible for this to be set without another thread being stuck in ProcessSegmentsInternal as well.

The only other stack where there's anything related to DataPipe appears to be 'IPC I/O Child', and that one doesn't have ProcessSegmentsInternal on the stack.

Not sure how this could happen unless something is clobbering the mProcessingSegment boolean. Marking this as a sec in case it's exploitable.

I also looked into bug 2005113, and have left some comments there about some potential race conditions. I'm unsure if those could cause this particular memory corruption, but it's possible.

Comment 6

[Daniel Veditz [:dveditz]]

This is not a new issue but it flew under the radar for a long time before spiking on the release channel, but only in Fenix.

But it spiked across versions so the spike is probably related to a change in some popular site. Unfortunately Fenix doesn't report URLs

Comment 7

[Gabriele Svelto [:gsvelto]]

I poked at a few minidumps in the hope of finding a hint of what site might be causing this but couldn't find anything relevant.

Comment 8

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

Is there any way to see from the crash reports that fission is enabled? I wonder this is some fallout from fission being recently enabled. (Although that's 146/147+, it seems we also did some A/B test for older builds)

Comment 9

[Kagami Rosylight 🏳️‍🌈🏳️‍⚧️ [:saschanaz] (they/them)]

FWIW, the only consumers of InputToReadableStreamAlgorithms are Fetch/Blob/WebTransport right now. CC'ing :jesup for WT part.

(But likely Fetch, as bug 2005113 is about? Kinda hard to believe WebTransport has such high usage from random small websites and can cause a top crash)

Comment 10

[BugBot [:suhaib / :marco]]

The bug is marked as tracked for firefox147 (release). However, the bug still isn't assigned.

:jstutte, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Comment 11

[Jens Stutte [:jstutte]]

(In reply to Kagami Rosylight [:saschanaz] (they/them) from comment #8)

Is there any way to see from the crash reports that fission is enabled? I wonder this is some fallout from fission being recently enabled. (Although that's 146/147+, it seems we also did some A/B test for older builds)

FWIW, this is a crashreport from Windows with the DOMFissionEnabled: 1 annotation.

There I see two threads inside DataPipeBase::ProcessSegmentsInternal:

Main Thread
0 	xul.dll 	mozilla::ipc::data_pipe_detail::DataPipeBase::ProcessSegmentsInternal(unsigned int, mozilla::FunctionRef<nsresult (mozilla::Span<char, 18446744073709551615>, unsigned int, unsigned int*)>, unsigned int*) 	ipc/glue/DataPipe.cpp:317 	context
1 	xul.dll 	mozilla::ipc::DataPipeReceiver::ReadSegments(nsresult (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) 	ipc/glue/DataPipe.cpp:644 	inlined
1 	xul.dll 	mozilla::ipc::DataPipeReceiver::Read(char*, unsigned int, unsigned int*) 	ipc/glue/DataPipe.cpp:633 	cfi
2 	xul.dll 	mozilla::dom::InputStreamHolder::Read(char*, unsigned int, unsigned int*) 	dom/streams/UnderlyingSourceCallbackHelpers.h:197 	inlined
2 	xul.dll 	mozilla::dom::InputToReadableStreamAlgorithms::PullFromInputStream(JSContext*, unsigned long long, mozilla::ErrorResult&) 	dom/streams/UnderlyingSourceCallbackHelpers.cpp:467 	cfi
3 	xul.dll 	mozilla::dom::InputToReadableStreamAlgorithms::OnInputStreamReady(nsIAsyncInputStream*) 	dom/streams/UnderlyingSourceCallbackHelpers.cpp:313 	cfi
4 	xul.dll 	mozilla::dom::InputStreamHolder::OnInputStreamReady(nsIAsyncInputStream*) 	dom/streams/UnderlyingSourceCallbackHelpers.cpp:229 	cfi
5 	xul.dll 	mozilla::ipc::DataPipeReceiver::AsyncWait::<lambda_8>::operator()() const 	ipc/glue/DataPipe.cpp:671 	inlined
5 	xul.dll 	NS_NewCancelableRunnableFunction<`lambda at /builds/worker/checkouts/gecko/ipc/glue/DataPipe.cpp:667:23'>::FuncCancelableRunnable::Run() 	xpcom/threads/nsThreadUtils.h:637 	cfi
6 	xul.dll 	mozilla::RunnableTask::Run() 	xpcom/threads/TaskController.cpp:705 	inlined

StreamTrans #1
0 	ntdll.dll 	ZwWaitForAlertByThreadId 		context
1 	ntdll.dll 	RtlAcquireSRWLockExclusive 		cfi
2 	xul.dll 	mozilla::OffTheBooksMutex::Lock() 	xpcom/threads/Mutex.h:66 	inlined
2 	xul.dll 	mozilla::detail::BaseAutoUnlock<mozilla::Mutex&>::~BaseAutoUnlock() 	xpcom/threads/Mutex.h:342 	inlined
2 	xul.dll 	mozilla::ipc::data_pipe_detail::DataPipeBase::ProcessSegmentsInternal(unsigned int, mozilla::FunctionRef<nsresult (mozilla::Span<char, 18446744073709551615>, unsigned int, unsigned int*)>, unsigned int*) 	ipc/glue/DataPipe.cpp:367 	cfi
3 	xul.dll 	mozilla::ipc::DataPipeReceiver::ReadSegments(nsresult (*)(nsIInputStream*, void*, char const*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) 	ipc/glue/DataPipe.cpp:644 	inlined
3 	xul.dll 	mozilla::ipc::DataPipeReceiver::Read(char*, unsigned int, unsigned int*) 	ipc/glue/DataPipe.cpp:633 	cfi
4 	xul.dll 	nsStreamCopierOB::FillOutputBuffer(nsIOutputStream*, void*, char*, unsigned int, unsigned int, unsigned int*) 	xpcom/io/nsStreamUtils.cpp:558 	cfi
5 	xul.dll 	nsPipeOutputStream::WriteSegments(nsresult (*)(nsIOutputStream*, void*, char*, unsigned int, unsigned int, unsigned int*), void*, unsigned int, unsigned int*) 	xpcom/io/nsPipe3.cpp:1713 	cfi
6 	xul.dll 	nsStreamCopierOB::DoCopy(nsresult*, nsresult*) 	xpcom/io/nsStreamUtils.cpp:575 	cfi
7 	xul.dll 	nsAStreamCopier::Process() 	xpcom/io/nsStreamUtils.cpp:304 	inlined
7 	xul.dll 	nsAStreamCopier::Run() 	xpcom/io/nsStreamUtils.cpp:431 	cfi
8 	xul.dll 	nsThreadPool::Run() 	xpcom/threads/nsThreadPool.cpp:441 	cfi

Apparently the nsStreamCopierOB::DoCopy started on the STS pool and just finished the processing loop and wants to re-acquire the (shared) mMutex which was already acquired by the main thread who finds mProcessingSegment true. I might be overlooking something, but up to nsStreamCopierOB::Run and/or the lambda inside DataPipeReceiver::AsyncWait I see no other synchronization mechanism that could avoid this once those two runnables are dispatched. So even if at the moment of dispatch we were in a synchronized state (though DataPipeBase::CheckStatus seems to not look at mProcessingSegment), we might still collide when we actually run, AFAICT.

There might be a higher level question why we see the same stream used in two different contexts. But the async dispatching of the actual processing might make become things parallel which seemed to be sequential at the call sites?

Comment 12

[BugBot [:suhaib / :marco]]

Based on the topcrash criteria, the crash signature linked to this bug is not a topcrash signature anymore.

For more information, please visit BugBot documentation.

Comment 13

[Jens Stutte [:jstutte]]

(In reply to BugBot [:suhaib / :marco/ :calixte] from comment #10)

The bug is marked as tracked for firefox147 (release). However, the bug still isn't assigned.

:jstutte, could you please find an assignee for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit BugBot documentation.

Eden is working on bug Bug 2005113, and there is a good chance they are related.

Said that, it seems like on 148 beta the signature has gone, while 148 nightly still showed it, but stops with 149. Did we land something early in 149 and uplifted it ?

Comment 14

[Ryan VanderMeulen [:RyanVM]]

Volume has dropped off on 147 also. I wonder a site change mitigated it.