Skip to content

Commit

Permalink
Bug 1873790 - Ensure that we release CanvasChild::DataShmemHolder on …
Browse files Browse the repository at this point in the history
…main thread. r=gfx-reviewers,lsalzman

CanvasChild is not thread-safe and can only be interacted with on its
owning thread.

Differential Revision: https://phabricator.services.mozilla.com/D198347
  • Loading branch information
aosmond committed Jan 12, 2024
1 parent 7ee5c5d commit f2b97f1
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 8 deletions.
9 changes: 9 additions & 0 deletions dom/canvas/crashtests/1873790.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<script>
document.addEventListener("DOMContentLoaded", async () => {
let a = document.createElementNS("http://www.w3.org/1999/xhtml", "canvas")
let bmp = await self.createImageBitmap(a.getContext("2d"), 100, -187, -214, 57, { })
let chan = new MessageChannel()
const worker = new Worker("none", { })
worker.postMessage([bmp, chan.port1], [bmp, chan.port1])
})
</script>
1 change: 1 addition & 0 deletions dom/canvas/crashtests/crashtests.list
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,4 @@ pref(gfx.offscreencanvas.enabled,true) load 1771007-1.html
pref(dom.webgpu.enabled,true) load 1816140.html
load 1849704-1.html
load 1857986.html
load 1873790.html
22 changes: 14 additions & 8 deletions gfx/layers/ipc/CanvasChild.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -435,19 +435,25 @@ already_AddRefed<gfx::DataSourceSurface> CanvasChild::GetDataSurface(

RefPtr<gfx::DataSourceSurface> dataSurface =
gfx::Factory::CreateWrappingDataSourceSurface(
data, stride, ssSize, ssFormat,
[](void* aClosure) {
auto* shmemHolder = static_cast<DataShmemHolder*>(aClosure);
shmemHolder->canvasChild->ReturnDataSurfaceShmem(
shmemHolder->shmem.forget());
delete shmemHolder;
},
closure);
data, stride, ssSize, ssFormat, ReleaseDataShmemHolder, closure);

mRecorder->WaitForCheckpoint(checkpoint);
return dataSurface.forget();
}

/* static */ void CanvasChild::ReleaseDataShmemHolder(void* aClosure) {
if (!NS_IsMainThread()) {
NS_DispatchToMainThread(NS_NewRunnableFunction(
"CanvasChild::ReleaseDataShmemHolder",
[aClosure]() { ReleaseDataShmemHolder(aClosure); }));
return;
}

auto* shmemHolder = static_cast<DataShmemHolder*>(aClosure);
shmemHolder->canvasChild->ReturnDataSurfaceShmem(shmemHolder->shmem.forget());
delete shmemHolder;
}

already_AddRefed<gfx::SourceSurface> CanvasChild::WrapSurface(
const RefPtr<gfx::SourceSurface>& aSurface, int64_t aTextureId) {
NS_ASSERT_OWNINGTHREAD(CanvasChild);
Expand Down
7 changes: 7 additions & 0 deletions gfx/layers/ipc/CanvasChild.h
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,13 @@ class CanvasChild final : public PCanvasChild, public SupportsWeakPtr {
void ReturnDataSurfaceShmem(
already_AddRefed<ipc::SharedMemoryBasic> aDataSurfaceShmem);

struct DataShmemHolder {
RefPtr<ipc::SharedMemoryBasic> shmem;
RefPtr<CanvasChild> canvasChild;
};

static void ReleaseDataShmemHolder(void* aClosure);

void DropFreeBuffersWhenDormant();

static const uint32_t kCacheDataSurfaceThreshold = 10;
Expand Down

0 comments on commit f2b97f1

Please sign in to comment.