From 350fc699a1e59ef8b563393e2c52ba4979320bce Mon Sep 17 00:00:00 2001 From: Swung0x48 Date: Fri, 2 Oct 2026 09:18:05 -0400 Subject: [PATCH] [Fix] (SharedImage): create dma-buf wl_buffers without create_immed, drain the ring before teardown ops, and decline Magma records after the renderer is gone --- .../MG_Backend/DirectVulkan/DirectVulkan.cpp | 7 +++- .../DirectVulkan/Renderer/VkBufferManager.cpp | 36 +++++++++++-------- MobileGL/MG_Impl/EGLImpl/WaylandWindow.cpp | 34 ++++++++++-------- MobileGL/MG_Remote/Server/ServerLoop.cpp | 11 ++++++ MobileGL/build49.log | 1 + 5 files changed, 59 insertions(+), 30 deletions(-) create mode 100644 MobileGL/build49.log diff --git a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp index ef62d30f..bc050bc2 100644 --- a/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/DirectVulkan.cpp @@ -1760,7 +1760,12 @@ namespace MobileGL::MG_Backend::DirectVulkan { } void Present() { - MOBILEGL_ASSERT(pVulkanRenderer, "DirectVulkan::Present called with null VulkanRenderer"); + // A served session's renderer is gone once its client released its resources; a present + // that still reaches it has nothing to show and is dropped rather than dereferenced. + if (!pVulkanRenderer) { + MGLOG_E_ONCE("DirectVulkan: a present reached a session whose renderer is already released; dropped"); + return; + } pVulkanRenderer->Present(); // THE frame boundary for the MGPipe counters, at the backend entry point rather // than inside VulkanRenderer::Present: that function has an early return for the diff --git a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp index 0051a166..96d96fe6 100644 --- a/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp +++ b/MobileGL/MG_Backend/DirectVulkan/Renderer/VkBufferManager.cpp @@ -181,34 +181,39 @@ namespace MobileGL::MG_Backend::DirectVulkan { }; #if MOBILEGL_BUILD_DISAGGREGATED - VkBufferManager& WireManager() { - if (g_activeBufferManager.Get() == nullptr) { - MGLOG_F("Magma: Fatal{ResourceUnavailable, \"wire-buffer-manager\"}"); - std::abort(); + // The session's manager, or null once its renderer is gone - a client's records can still + // arrive after its eglTerminate released it (or, surfaceless, before its first surface built + // one). Such a record is declined with one line rather than taking the whole server down. + VkBufferManager* WireManager(const char* op) { + VkBufferManager* manager = g_activeBufferManager.Get(); + if (manager == nullptr) { + MGLOG_E_ONCE("Magma: a wire buffer %s reached a session with no renderer; declined", op); } - return *g_activeBufferManager.Get(); + return manager; } const MG_Pipe::MGPipeResourceOps g_vulkanWireResourceOps = { - .Create = [](auto res, const auto& desc) { WireManager().CreateWireBuffer(res, desc); }, + .Create = [](auto res, const auto& desc) { if (auto* m = WireManager("create")) m->CreateWireBuffer(res, desc); }, .Respecify = [](auto res, const auto& desc, const void* bytes) { - WireManager().RespecifyWireBuffer(res, desc, bytes); + if (auto* m = WireManager("respecify")) m->RespecifyWireBuffer(res, desc, bytes); }, .SubData = [](auto res, const auto& record, const void* bytes) { - WireManager().WriteWireBuffer(res, MG_Pipe::MGPipeSubDataBufferOffset(record), - MG_Pipe::MGPipeSubDataBufferSize(record), bytes); + if (auto* m = WireManager("write")) + m->WriteWireBuffer(res, MG_Pipe::MGPipeSubDataBufferOffset(record), + MG_Pipe::MGPipeSubDataBufferSize(record), bytes); }, .SubDataResident = [](auto res, const auto& record, const void* bytes) { - WireManager().WriteWireBuffer(res, MG_Pipe::MGPipeSubDataBufferOffset(record), - MG_Pipe::MGPipeSubDataBufferSize(record), bytes); + if (auto* m = WireManager("write")) + m->WriteWireBuffer(res, MG_Pipe::MGPipeSubDataBufferOffset(record), + MG_Pipe::MGPipeSubDataBufferSize(record), bytes); }, .FlushRange = [](auto res, const auto& record, const void* bytes) { - WireManager().FlushWireBuffer(res, record.Offset, record.Size, bytes); + if (auto* m = WireManager("flush")) m->FlushWireBuffer(res, record.Offset, record.Size, bytes); }, .Readback = [](auto res, const auto& record) { - WireManager().ReadbackWireBuffer(res, record.Offset, record.Size); + if (auto* m = WireManager("readback")) m->ReadbackWireBuffer(res, record.Offset, record.Size); }, - .Destroy = [](auto res) { WireManager().DestroyWireBuffer(res); }, + .Destroy = [](auto res) { if (auto* m = WireManager("destroy")) m->DestroyWireBuffer(res); }, // Split mapping remains T2: the client owns its map and pushes exact // modified ranges. A server pointer is never donated across the wire. .MapPersistent = [](MG_Pipe::MGPipeHandle, Uint64, const void*) -> void* { return nullptr; }, @@ -217,7 +222,8 @@ namespace MobileGL::MG_Backend::DirectVulkan { // P11 B2 (T0): the one door a server pointer is still never donated through stays shut; // what T0 adds is the other direction - the CLIENT's pages imported as the store. .ImportExternal = [](MG_Pipe::MGPipeHandle res, void* ahb, Uint64 size) -> Bool { - return WireManager().ImportWireBuffer(res, ahb, size); + auto* m = WireManager("import"); + return m != nullptr && m->ImportWireBuffer(res, ahb, size); }, .SelfTestExternal = [](char* why, Uint64 whyBytes) -> Bool { return VkBufferManager::SelfTestWireImport(why, whyBytes); diff --git a/MobileGL/MG_Impl/EGLImpl/WaylandWindow.cpp b/MobileGL/MG_Impl/EGLImpl/WaylandWindow.cpp index d108ec88..d0d987de 100644 --- a/MobileGL/MG_Impl/EGLImpl/WaylandWindow.cpp +++ b/MobileGL/MG_Impl/EGLImpl/WaylandWindow.cpp @@ -129,7 +129,7 @@ namespace MobileGL::MG_Impl::EGLImpl::Wayland { constexpr uint32_t kDmabufCreateParams = 1; constexpr uint32_t kParamsDestroy = 0; constexpr uint32_t kParamsAdd = 1; - constexpr uint32_t kParamsCreateImmed = 3; // since version 2 + constexpr uint32_t kParamsCreate = 2; // The highest linux-dmabuf version spoken here: 4 adds feedback objects this never asks for. constexpr uint32_t kDmabufMaxVersion = 3; // DRM fourccs of the server's shared images: R, G, B, A in memory - GL's RGBA8, so the @@ -268,8 +268,13 @@ namespace MobileGL::MG_Impl::EGLImpl::Wayland { void (*const kDmabufListener[])(void) = {reinterpret_cast(&OnDmabufFormat), reinterpret_cast(&OnDmabufModifier)}; - void OnParamsCreated(void*, wl_proxy*, wl_proxy*) {} - void OnParamsFailed(void* data, wl_proxy*) { *static_cast(data) = true; } + // The answer to a `create`: the new wl_buffer, or `failed`. + struct ParamsOutcome { + wl_proxy* buffer = nullptr; + Bool failed = false; + }; + void OnParamsCreated(void* data, wl_proxy*, wl_proxy* buffer) { static_cast(data)->buffer = buffer; } + void OnParamsFailed(void* data, wl_proxy*) { static_cast(data)->failed = true; } void (*const kParamsListener[])(void) = {reinterpret_cast(&OnParamsCreated), reinterpret_cast(&OnParamsFailed)}; @@ -422,9 +427,10 @@ namespace MobileGL::MG_Impl::EGLImpl::Wayland { buffer = DmabufBuffer{}; } - // A shared image of the window's size, as a wl_buffer. create_immed plus a round trip - // rather than a bare create_immed: a compositor that refuses the import says so with - // `failed`, and attaching that buffer would be a protocol error, fatal to the connection. + // A shared image of the window's size, as a wl_buffer. `create` and a round trip, never + // create_immed: a compositor that cannot import the buffer answers create_immed with a + // protocol error, fatal to the application's whole connection, where `create` answers + // `failed` and the window simply stays on wl_shm. Bool Allocate(DmabufBuffer& buffer) { const Api& api = WaylandApi(); const LinuxDmabufProtocol* protocol = LinuxDmabuf(); @@ -455,19 +461,20 @@ namespace MobileGL::MG_Impl::EGLImpl::Wayland { dropImage(); return false; } - Bool failed = false; - api.addListener(params, const_cast(kParamsListener), &failed); + ParamsOutcome outcome; + api.addListener(params, const_cast(kParamsListener), &outcome); // The descriptor is duplicated into the request, so it is closed below either way. api.marshalFlags(params, kParamsAdd, nullptr, version, 0, image.Fd, 0u, image.Offset, image.Stride, static_cast(image.Modifier >> 32), static_cast(image.Modifier & 0xffffffffu)); - wl_proxy* wlBuffer = api.marshalFlags(params, kParamsCreateImmed, api.bufferInterface, version, 0, nullptr, - width, height, fourcc, 0u); + api.marshalFlags(params, kParamsCreate, nullptr, version, 0, width, height, fourcc, 0u); const Bool settled = api.roundtripQueue(display, queue) >= 0; api.marshalFlags(params, kParamsDestroy, nullptr, version, kMarshalFlagDestroy); - if (!settled || failed || wlBuffer == nullptr) { + wl_proxy* wlBuffer = outcome.buffer; + if (!settled || outcome.failed || wlBuffer == nullptr) { MGLOG_E("Wayland: the compositor refused a %dx%d linux-dmabuf buffer", width, height); - if (wlBuffer) api.marshalFlags(wlBuffer, kBufferDestroy, nullptr, version, kMarshalFlagDestroy); + if (wlBuffer) + api.marshalFlags(wlBuffer, kBufferDestroy, nullptr, api.getVersion(wlBuffer), kMarshalFlagDestroy); dropImage(); return false; } @@ -536,8 +543,7 @@ namespace MobileGL::MG_Impl::EGLImpl::Wayland { api.marshalFlags(registry, kRegistryBind, api.shmInterface, 1, 0, name, "wl_shm", 1u, nullptr); return; } - // create_immed is version 2's: an older linux-dmabuf is left unbound. - if (impl->dmabufWanted && impl->dmabuf == nullptr && version >= 2 && + if (impl->dmabufWanted && impl->dmabuf == nullptr && std::strcmp(interface, "zwp_linux_dmabuf_v1") == 0) { const LinuxDmabufProtocol* protocol = LinuxDmabuf(); if (protocol == nullptr) return; diff --git a/MobileGL/MG_Remote/Server/ServerLoop.cpp b/MobileGL/MG_Remote/Server/ServerLoop.cpp index c4d8f425..2f562b88 100644 --- a/MobileGL/MG_Remote/Server/ServerLoop.cpp +++ b/MobileGL/MG_Remote/Server/ServerLoop.cpp @@ -848,6 +848,17 @@ namespace MobileGL::MG_Remote::Server { m_controlDone.notify_all(); return true; } + // A TEARDOWN ORDERS AFTER THE RECORDS ITS CLIENT PUBLISHED BEFORE ASKING FOR IT. The loop + // pumps a posted frame before it drains the ring, so an eglTerminate (or a surface or + // context release) could otherwise run ahead of that client's last swap still queued in + // the ring - and the swap then reaches a backend whose renderer the release destroyed. Not + // on a full SEG_EVENT, for the loop's own reason: the drain would park inside a post. + if ((frame.kind == SurfaceControlOp::ReleaseResources || frame.kind == SurfaceControlOp::ReleaseSurface || + frame.kind == SurfaceControlOp::DestroyContext) && + m_session != nullptr && m_session->DataLink() != nullptr && + m_session->DataLink()->Signals().EventRingFull->load(std::memory_order_acquire) == 0) { + DrainRing(); + } // The dispatch runs OUTSIDE the lock, exactly as the old work(user) did, and fills the // frame's reply half; the reply is then published back into the slot under the lock so // the poster's copy-out after m_controlDone sees it. diff --git a/MobileGL/build49.log b/MobileGL/build49.log new file mode 100644 index 00000000..2d401ad7 --- /dev/null +++ b/MobileGL/build49.log @@ -0,0 +1 @@ +bash: build-android.sh: No such file or directory