1879833 - Crash in [@ mozilla::SupportsThreadSafeWeakPtr<T>::refCount]

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

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

8 crashes from 3+ Firefox installs, all with 123 beta or 124 Nightly builds on Linux or macOS

All reports have a use-after-address.

Crash report: https://crash-stats.mozilla.org/report/index/e1d23cab-d7e2-4f19-8a8e-6e3260240205

Reason: SIGSEGV / SI_KERNEL

Top 10 frames of crashing thread:

0  libxul.so  RefPtr<mozilla::detail::ThreadSafeWeakReference>::get const  mfbt/RefPtr.h:314
0  libxul.so  RefPtr<mozilla::detail::ThreadSafeWeakReference>::operator-> const  mfbt/RefPtr.h:344
0  libxul.so  mozilla::SupportsThreadSafeWeakPtr<mozilla::gfx::SourceSurface>::refCount const  mfbt/ThreadSafeWeakPtr.h:189
0  libxul.so  mozilla::SupportsThreadSafeWeakPtr<mozilla::gfx::SourceSurface>::hasOneRef const  mfbt/ThreadSafeWeakPtr.h:190
0  libxul.so  mozilla::gfx::DrawTargetSkia::MarkChanged  gfx/2d/DrawTargetSkia.cpp:2067
0  libxul.so  mozilla::gfx::DrawTargetSkia::DetachAllSnapshots  gfx/2d/DrawTargetSkia.h:141
1  libxul.so  mozilla::layers::TextureClient::Unlock  gfx/layers/client/TextureClient.cpp:811
2  libxul.so  mozilla::layers::TextureClientAutoLock::~TextureClientAutoLock  gfx/layers/client/TextureClient.h:834
2  libxul.so  mozilla::detail::MaybeStorage<mozilla::layers::TextureClientAutoLock, false>::~MaybeStorage  mfbt/Maybe.h:269
2  libxul.so  mozilla::layers::PersistentBufferProviderShared::BorrowDrawTarget  gfx/layers/PersistentBufferProvider.cpp:566

Comment 1

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

Stack is corrupted for every of the 8 crash reports.

Comment 2

[Bob Hood [:bhood]]

Lee, this appears to be happening in Skia. Any insights you might be able to provide?

Comment 3

[Lee Salzman [:lsalzman]]

Andrew, this looks like offscreen canvas is somehow dropping the DT while its still in use?

Comment 4

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

So I think this must have been a regression from bug 1870488 given the PersistentBufferProviderShared implications, although to be honest, I'm entirely sure how it works. There definitely seems to be some thread unsafe assumptions, but how that maps to an invalid DrawTarget, I'm not sure since everything seems to hold strong pointers. We shouldn't hit any UAF conditions as a result of it, but maybe.

With that said, I believe I must have fixed this when I landed bug 1855742 part 1, where I changed how the snapshot path worked because recordings made it very complicated. Two things fixed this. 1) I only handled the Skia surface on the owning thread, and 2) I made a copy of it. I don't actually remember my motivation for 2), but I clearly must have been hitting issues in CI, and didn't make the connection that this would be a problem for the existing code.

I'm a little nervous about requesting a release uplift to 123/124, but it has soaked for some time at least....

Comment 5

[Jim Mathies [:jimm]]

Tentatively marking as fixed in 125. No crashes in 125 yet according to crash stats.

Comment 6

[Daniel Veditz [:dveditz]]

Let's mark this tracking for Fx125 as a reminder to check back on crash-stats to see if comment 5 continues to hold

Comment 7

[Daniel Veditz [:dveditz]]

Still looks fixed