1530305 - Crash in [@ mozilla::ipc::FatalError | mozilla::ipc::IProtocol::HandleFatalError | <name omitted> | mozilla::ipc::WriteIPDLParam<T>]

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect, P2)

Description

[Gabriele Svelto [:gsvelto]]

This bug is for crash report bp-44b669cb-4552-4215-804c-262c90190224.

Top 10 frames of crashing thread:

0 XUL mozilla::ipc::FatalError ipc/glue/ProtocolUtils.cpp:259
1 XUL mozilla::ipc::IProtocol::HandleFatalError const ipc/glue/ProtocolUtils.cpp:440
2 XUL <name omitted> ipc/ipdl/LayersSurfaces.cpp:2872
3 XUL void mozilla::ipc::WriteIPDLParam<mozilla::RemoteVideoDataIPDL const&> ipc/ipdl/PRemoteDecoder.cpp:150
4 XUL void mozilla::ipc::WriteIPDLParam<mozilla::DecodedOutputIPDL const&> ipc/ipdl/PRemoteDecoder.cpp:630
5 XUL mozilla::PRemoteDecoderParent::SendOutput ipc/ipdl/PRemoteDecoderParent.cpp:119
6 XUL mozilla::RemoteVideoDecoderParent::ProcessDecodedData dom/media/ipc/RemoteVideoDecoder.cpp:199
7 XUL mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true>::ThenValue<mozilla::RemoteDecoderParent::RecvInput dom/media/ipc/RemoteDecoderParent.cpp:89
8 XUL mozilla::MozPromise<nsTArray<RefPtr<mozilla::MediaData> >, mozilla::MediaResult, true>::ThenValueBase::ResolveOrRejectRunnable::Run xpcom/threads/MozPromise.h:392
9 XUL mozilla::TaskQueue::Runner::Run xpcom/threads/TaskQueue.cpp:199

This seems like a new crash and there's some RDD entries on the stack. Tentatively filing it under Audio/Video: Playback.

Comment 1

[Andrew McCreight [:mccr8]]

The crash reason for these is:
MOZ_CRASH(IPC FatalError in the parent process!)

I'm not sure what that means.

It looks like this crash has become more frequently recently. It is the top OSX crash on the 3/13 Nightlies.

Comment 2

[Gabriele Svelto [:gsvelto]]

Most of the crashes have the IPCFatalErrorMsg annotation set to "unknown union type". It's coming from here:

https://searchfox.org/mozilla-central/source/__GENERATED__/ipc/ipdl/PRemoteDecoder.cpp#618

The DecoderOutputIPDL union in the message must contain an unknown type, maybe it's corrupted?

Comment 3

[Andrew McCreight [:mccr8]]

I've seen something like that before when somebody sent a union that wasn't fully initialized, but the only two places I see that create a DecoderOutputIPDL look okay to me.

Comment 4

[Andrew McCreight [:mccr8]]

This looks like a similar crash on Windows, with a more detailed stack:
bp-2ba68443-2d66-4afc-beee-30f5a0190312#tab-details

DecodedOutputIPDL is also on the stack, but there's actually some nested type BufferDescriptor that maybe it is hitting the error for. That's a field of the type SurfaceDescriptorBuffer.

The code around the SendOutput that is crashing is this:

SurfaceDescriptorBuffer sdBuffer;
Shmem buffer;
if (AllocShmem(image->GetDataSize(), Shmem::SharedMemory::TYPE_BASIC,
               &buffer) &&
    image->GetDataSize() == buffer.Size<uint8_t>()) {
   ...
}

RemoteVideoDataIPDL output(
    ...,
    video->mDisplay, image->GetSize(), sdBuffer, video->mFrameID);
Unused << SendOutput(output);

It looks like that if the |if (...)| code fails, then we send an uninitialized SurfaceDescriptorBuffer, which contains an uninitialized BufferDescriptor, which I think would hit the unknown type error. Failing to allocate shmem doesn't seem so unlikely.

Could you please take a look at this, Michael? It looks like a regression from bug 1500596. Thanks.

Comment 5

[Andrew McCreight [:mccr8]]

Adding a Windows signature.

Comment 6

[Michael Froman [:mjf]]

I thought I checked during initial dev work that we could send an empty SurfaceDescriptorBuffer, but maybe not. I will take a look.

Comment 7

[Michael Froman [:mjf]]

Also strange that the spike started around the beginning of March when bug 1500596 landed mid-Feb.

Comment 8

[Michael Froman [:mjf]]

(In reply to Michael Froman [:mjf] from comment #6)

I thought I checked during initial dev work that we could send an empty SurfaceDescriptorBuffer, but maybe not. I will take a look.

Confirmed that sending an empty SurfaceDescriptorBuffer will crash. Let me see what I can do to fix it.

Comment 9

[Michael Froman [:mjf]]

Attachment: Bug 1530305 - handle failure to alloc Shmem in RemoteVideoDecoderParent. r?jya!

• Modify ProcessDecodedData to return a MediaResult.
• RemoteDecoderParent::RecvInput and RemoteDecoderParent::RecvDrain
  both use error returned from ProcessDecodedData to call SendError.
• RemoteVideoDecoderParent and RemoteAudioDecoderParent both return
  MediaResult with NS_ERROR_OUT_OF_MEMORY if AllocShmem fails in
  ProcessDecodedData (or if the returned buffer size is less than
  the requested size).

Comment 10

[Michael Froman [:mjf]]

Turns out this is not a regression from Bug 1500596. This was lurking since the original RDD code landed. It would be interesting to know why we're seeing more frequent failures to alloc Shmem.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/beaf8e80224d
handle failure to alloc Shmem in RemoteVideoDecoderParent. r=jya

Comment 12

[Razvan Maries]

https://hg.mozilla.org/mozilla-central/rev/beaf8e80224d

Comment 13

[Michael Froman [:mjf]]

Reports of this crash appear to have stopped after build id 20190319095054, so I'll file an uplift request.

Comment 14

[Michael Froman [:mjf]]

Comment on attachment 9051896 [details]
Bug 1530305 - handle failure to alloc Shmem in RemoteVideoDecoderParent. r?jya!

Beta/Release Uplift Approval Request

• Feature/Bug causing the regression: Bug 1471535
• User impact if declined: Crash in RDD process when AllocShmem fails.
• 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): This is a fairly small change (localized to 6 files): Adds a return param to ProcessDecodedData and then returns an error if AllocShmem fails to avoid returning an uninitialized SurfaceDescriptorBuffer down in DecodedOutputIPDL.
• String changes made/needed: none

Comment 15

[Pascal Chevrel:pascalc]

Comment on attachment 9051896 [details]
Bug 1530305 - handle failure to alloc Shmem in RemoteVideoDecoderParent. r?jya!

Fix to our Vorbis support causing crashes which landed a week ago on Nightly which seems to have fixed the crashes since then. It's a low frequency crash but we are early in the Beta cycle and the patch is delimited and well understood, uplift approved for 67 beta 5, thanks.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/17ada400a1db