Bug 1798539 - Release temp shmem on Send-side after Ping response. r=gfx-reviewers,lsalzman

Originally, we special-cased shmems to try to send them directly
without a copy.
Unfortunately, ipc::Shmem doesn't support that, so we
allocated a new shmem on PWebGL and sent that.
However, we didn't include shmem-cleanup steps in RecvTexImage, so we
leaked the shmem.

We would be tempted to check inside the Desc in RecvTexImage and cleanup that
shmem, *however* RawTexImage from dtwebgl also uses this path, and its shmems
are UnsafeShmems, which are *not* supposed to be cleaned up.

Rather than adding e.g. a bool to differentiate them, let's just explicitly do
the cleanup in the caller that knows it needs to clean up.

One pattern I want to use more often for cross-process lifetimes is:
```
  foo = new RefcountedResource();
  SendUseAsync(foo);
  SendPing()->Then([foo]() {});
```

By the time we the promise from SendPing is resolved, we know the remote side is
done with `foo`, and so our trivial capturing lambda will take care of releasing
it on the Send side. It's straightforward and safe, and the lifetimes have one
extra return-trip of latency, but that's almost always fine.

Differential Revision: https://phabricator.services.mozilla.com/D160996
This commit is contained in:
Kelsey Gilbert 2022-11-02 05:22:09 +00:00
Родитель 1944552082
Коммит cde25793b6
5 изменённых файлов: 54 добавлений и 8 удалений

Просмотреть файл

@ -4314,6 +4314,8 @@ void ClientWebGLContext::TexImage(uint8_t funcDims, GLenum imageTarget,
// -
std::shared_ptr<webgl::RaiiShmem> tempShmem;
const bool doInlineUpload = !desc->sd;
// Why always de-inline SDs here?
// 1. This way we always send SDs down the same handling path, which
@ -4347,20 +4349,34 @@ void ClientWebGLContext::TexImage(uint8_t funcDims, GLenum imageTarget,
// transport.
if (pShmem) {
MOZ_ASSERT(desc->sd);
const auto byteCount = pShmem->Size<uint8_t>();
const auto* const src = pShmem->get<uint8_t>();
mozilla::ipc::Shmem shmemForResend;
if (!child->AllocShmem(byteCount, &shmemForResend)) {
const auto srcBytes = ShmemRange<uint8_t>(*pShmem);
tempShmem = std::make_shared<webgl::RaiiShmem>();
// We need Unsafe because we want to dictate when to destroy it from the
// client side.
*tempShmem = webgl::RaiiShmem::AllocUnsafe(child, srcBytes.length());
if (!*tempShmem) {
NS_WARNING("AllocShmem failed in TexImage");
return;
}
auto* const dst = shmemForResend.get<uint8_t>();
memcpy(dst, src, byteCount);
*pShmem = shmemForResend;
const auto dstBytes = ShmemRange<uint8_t>(tempShmem->Shmem());
Memcpy(&dstBytes, srcBytes.begin());
*pShmem = tempShmem->Shmem();
// Not Extract, because we free tempShmem manually below, after the remote
// side has finished executing SendTexImage.
}
(void)child->SendTexImage(static_cast<uint32_t>(level), respecFormat,
CastUvec3(offset), pi, std::move(*desc));
if (tempShmem) {
const auto eventTarget = GetCurrentSerialEventTarget();
MOZ_ASSERT(eventTarget);
child->SendPing()->Then(eventTarget, __func__, [tempShmem]() {
// Cleans up when (our copy of) sendableShmem goes out of scope.
});
}
}
}

Просмотреть файл

@ -13,6 +13,7 @@ using mozilla::layers::SurfaceDescriptor from "mozilla/layers/LayersTypes.h";
using std::string from "string";
using mozilla::uvec2 from "mozilla/dom/WebGLIpdl.h";
using mozilla::uvec3 from "mozilla/dom/WebGLIpdl.h";
using mozilla::void_t from "mozilla/ipc/IPCCore.h";
using mozilla::webgl::CompileResult from "mozilla/dom/WebGLIpdl.h";
using mozilla::webgl::ContextLossReason from "mozilla/dom/WebGLIpdl.h";
using mozilla::webgl::FrontBufferSnapshotIpc from "mozilla/dom/WebGLIpdl.h";
@ -56,7 +57,7 @@ parent:
// -
async DispatchCommands(Shmem commands, uint64_t size);
async Ping() returns (void_t ok);
async TexImage(uint32_t level, uint32_t respecFormat, uvec3 offset,
PackingInfo pi, TexUnpackBlobDesc src);

Просмотреть файл

@ -34,6 +34,13 @@ class RaiiShmem final {
return {allocator, shmem};
}
static RaiiShmem AllocUnsafe(mozilla::ipc::IProtocol* const allocator,
const size_t size) {
mozilla::ipc::Shmem shmem;
if (!allocator->AllocUnsafeShmem(size, &shmem)) return {};
return {allocator, shmem};
}
// -
RaiiShmem() = default;

Просмотреть файл

@ -35,6 +35,12 @@ class WebGLParent : public PWebGLParent, public SupportsWeakPtr {
using IPCResult = mozilla::ipc::IPCResult;
template <class ResolveT>
IPCResult RecvPing(const ResolveT& Resolve) {
Resolve(void_t{});
return IPC_OK();
}
IPCResult RecvDispatchCommands(mozilla::ipc::Shmem&&, uint64_t);
IPCResult RecvTexImage(uint32_t level, uint32_t respecFormat,
const uvec3& offset, const webgl::PackingInfo&,

Просмотреть файл

@ -878,6 +878,11 @@ class RawBuffer final {
RawBuffer& operator=(RawBuffer&&) = default;
};
template <class T>
inline Range<T> ShmemRange(const mozilla::ipc::Shmem& shmem) {
return {shmem.get<T>(), shmem.Size<T>()};
}
// -
template <typename C, typename K>
@ -1166,6 +1171,17 @@ inline void Memcpy(const RangedPtr<uint8_t>& destBytes,
memcpy(destBytes.get(), srcBytes.get(), byteSize);
}
template <class T, class U>
inline void Memcpy(const Range<T>* const destRange,
const RangedPtr<U>& srcBegin) {
Memcpy(destRange->begin(), srcBegin, destRange->length());
}
template <class T, class U>
inline void Memcpy(const RangedPtr<T>* const destBegin,
const Range<U>& srcRange) {
Memcpy(destBegin, srcRange->begin(), srcRange->length());
}
// -
namespace webgl {