1870683 - Crash in [@ mozilla::layers::CanvasTranslator::SetDataSurfaceBuffer]

Status: RESOLVED FIXED

(Core :: Graphics: Canvas2D, defect, P3)

Description

[Andrew Osmond [:aosmond] (he/him)]

Crash report: https://crash-stats.mozilla.org/report/index/ba288d38-04f2-4080-9d08-d90ab0231217

MOZ_CRASH Reason: MOZ_RELEASE_ASSERT(mHeader->readerState == State::Paused)

Top 10 frames of crashing thread:

0  xul.dll  mozilla::layers::CanvasTranslator::SetDataSurfaceBuffer  gfx/layers/ipc/CanvasTranslator.cpp:246
1  xul.dll  mozilla::detail::RunnableMethodArguments<mozilla::UniquePtr<void*, mozilla::detail::FileHandleDeleter>&&, unsigned long long>::apply<mozilla::layers::CanvasTranslator, void  const  xpcom/threads/nsThreadUtils.h:1213
1  xul.dll  std::invoke  /builds/worker/fetches/vs/VC/Tools/MSVC/14.29.30133/include/type_traits:1534
1  xul.dll  std::_Apply_impl  /builds/worker/fetches/vs/VC/Tools/MSVC/14.29.30133/include/tuple:974
1  xul.dll  std::apply  /builds/worker/fetches/vs/VC/Tools/MSVC/14.29.30133/include/tuple:979
1  xul.dll  mozilla::detail::RunnableMethodArguments<mozilla::UniquePtr<void*, mozilla::detail::FileHandleDeleter>&&, unsigned long long>::apply  xpcom/threads/nsThreadUtils.h:1162
1  xul.dll  mozilla::detail::RunnableMethodImpl<mozilla::layers::CanvasTranslator*, void   xpcom/threads/nsThreadUtils.h:1213
2  xul.dll  nsThread::ProcessNextEvent  xpcom/threads/nsThread.cpp:1193
2  xul.dll  NS_ProcessNextEvent  xpcom/threads/nsThreadUtils.cpp:480
3  xul.dll  mozilla::ipc::MessagePumpForNonMainThreads::Run  ipc/glue/MessagePump.cpp:300

Comment 1

[Andrew Osmond [:aosmond] (he/him)]

Bug 1869659 landed to fix this, but we are still seeing some crashes with this signature.

Comment 2

[Andrew Osmond [:aosmond] (he/him)]

I wonder if we hit some sort of race and set the state to Processing when we should have remained Paused?
https://searchfox.org/mozilla-central/rev/91cc8848427fdbbeb324e6ca56a0d08d32d3c308/gfx/layers/ipc/CanvasTranslator.cpp#445

Comment 3

[Andrew Osmond [:aosmond] (he/him)]

I believe this has a similar root cause as bug 1869661. We failed to initialize the CanvasChild, and it appears we can in theory create a RecordedTextureData with it and try to read it back.

Comment 4

[Andrew Osmond [:aosmond] (he/him)]

Attachment: Bug 1870683 - Ensure CanvasChild::EnsureDataSurfaceShmem checks for uninitialized state.

If we fail to create CanvasChild::mRecorder, we shouldn't attempt to
readback a recording surface that never got created.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/986bbc7b49b1
Ensure CanvasChild::EnsureDataSurfaceShmem checks for uninitialized state. r=gfx-reviewers,lsalzman

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/986bbc7b49b1

Comment 7

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

Set release status flags based on info from the regressing bug 1863914

Comment 8

[Donal Meehan [:dmeehan]]

:aosmond could you nominate this for a beta uplift?

Comment 9

[Andrew McCreight [:mccr8]]

Maybe this was expected, but the crashes on Nightly have continued even after the patch landed. The assertion is the same, too.

Comment 10

[Andrew Osmond [:aosmond] (he/him)]

I don't have an explanation for the current crashes.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Looks like bug 1871811 resolved the remaining crashes under this signature.

Comment 12

[Andrew Osmond [:aosmond] (he/him)]

Comment on attachment 9369279 [details]
Bug 1870683 - Ensure CanvasChild::EnsureDataSurfaceShmem checks for uninitialized state.

Beta/Release Uplift Approval Request

• User impact if declined: Moderate volume content process crash
• Is this code covered by automated tests?: Yes
• 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): Trivial check to fail if we have a null recorder
• String changes made/needed:
• Is Android affected?: No

Comment 13

[Donal Meehan [:dmeehan]]

Comment on attachment 9369279 [details]
Bug 1870683 - Ensure CanvasChild::EnsureDataSurfaceShmem checks for uninitialized state.

Approved for 122.0b6

Comment 14

[Pulsebot]

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