1785940 - Crash in [@ mozilla::ipc::FatalError | mozilla::ipc::IProtocol::HandleFatalError | IPC::ParamTraits<mozilla::dom::IPCBlob>::Write]

Status: RESOLVED FIXED

(Core :: DOM: Content Processes, defect)

Description

[Sean Feng [:sefeng]]

Crash report: https://crash-stats.mozilla.org/report/index/ecb8fba7-1155-4177-8970-a1f710220615

Reason: EXCEPTION_BREAKPOINT

Top 10 frames of crashing thread:

0 xul.dll mozilla::ipc::FatalError ipc/glue/ProtocolUtils.cpp:170
1 xul.dll mozilla::ipc::IProtocol::HandleFatalError const ipc/glue/ProtocolUtils.cpp:403
2 xul.dll static IPC::ParamTraits<mozilla::dom::IPCBlob>::Write ipc/ipdl/IPCBlob.cpp:449
3 xul.dll static mozilla::dom::PFileCreatorParent::Send__delete__ ipc/ipdl/PFileCreatorParent.cpp:76
4 xul.dll mozilla::detail::RunnableFunction<`lambda at /builds/worker/checkouts/gecko/dom/file/ipc/FileCreatorParent.cpp:47:61'>::Run xpcom/threads/nsThreadUtils.h:532
5 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1174
6 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:330
7 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:373
8 xul.dll MessageLoop::Run ipc/chromium/src/base/message_loop.cc:355
9 xul.dll static nsThread::ThreadFunc xpcom/threads/nsThread.cpp:378

Comment 1

[Jens Stutte [:jstutte]]

I am not sure if bug 1773088 really matters here, as we see very few instances also before, but the uptick in nightly could be somewhat correlated and/or you might be a good person with recent context.

Comment 2

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

The crash from comment 0 appears to be from the windows error reporter, so doesn't have useful metadata, however some other crashes like https://crash-stats.mozilla.org/report/index/1d449e45-2ba7-487c-b872-b9ad00220906 has more useful metadata.

The IPCFatalErrorMessage is "unknown union type", which is generated when serializing an IPDL union which has an unknown variant, usually the T__None variant used for a default-constructed uninitialized IPDL variant.

Unfortunately, we don't include the specific type which failed in the error message, and it appears that there has been quite a bit of inlining in the stack (IPCBlob isn't an IPDL union, but contains many different IPDL unions). I've filed bug 1789708 to potentially improve the errors there.

Downloading the minidump and opening it in visual studio allowed me to see one extra inline frame:

[Inline Frame] xul.dll!IPC::WriteParam(IPC::MessageWriter * writer, const mozilla::RemoteLazyStream & p)

This suggests to me that the RemoteLazyStream enum is the one which is not fully initialized, suggesting that the IPCBlobUtils::Serialize call returned a successful nsresult without initializing the ipcBlob outparameter somehow: https://searchfox.org/mozilla-central/rev/2d65dbd85ca11309dc0c485a63393110ef800083/dom/file/ipc/FileCreatorParent.cpp#50. Inspecting the source, the only place this is possible is due to this mistake: https://searchfox.org/mozilla-central/rev/2d65dbd85ca11309dc0c485a63393110ef800083/dom/file/ipc/IPCBlobUtils.cpp#126, which returns rv.StealNSResult() when it is known to be successful without initializing inputStream. I'll put up a patch to fix that issue. The only place which this can happen is if RemoteLazyInputStreamStorage::Get() returns an error, whicH I believe can only happen during shutdown (https://searchfox.org/mozilla-central/rev/2d65dbd85ca11309dc0c485a63393110ef800083/dom/file/ipc/RemoteLazyInputStream.cpp#190-195).

Comment 3

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

Attachment: Bug 1785940 - Fix buggy error path in IPCBlobUtils::Serialize, r=asuth!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9cb186ec8315
Fix buggy error path in IPCBlobUtils::Serialize, r=asuth

Comment 5

[Cristina Cozmuta (:CrissCozmuta)]

https://hg.mozilla.org/mozilla-central/rev/9cb186ec8315

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:nika, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox105 to wontfix.

For more information, please visit auto_nag documentation.

Comment 7

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

Comment on attachment 9293550 [details]
Bug 1785940 - Fix buggy error path in IPCBlobUtils::Serialize, r=asuth!

Beta/Release Uplift Approval Request

• User impact if declined: crashes
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Straightforward single-line patch
• String changes made/needed: None
• Is Android affected?: No

Comment 8

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

Comment on attachment 9293550 [details]
Bug 1785940 - Fix buggy error path in IPCBlobUtils::Serialize, r=asuth!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: trivial patch to fix potential crashes
• User impact if declined: crashes
• Fix Landed on Version: 106
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): trivial single-line patch

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9293550 [details]
Bug 1785940 - Fix buggy error path in IPCBlobUtils::Serialize, r=asuth!

Approved for 105.0rc1 and 102.3esr.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/ab8f6f022201

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/44f41d16ca2a