From a3d7e70f41dd9054bd704cf6b9e4503645850ba1 Mon Sep 17 00:00:00 2001 From: Leask Wong Date: Sun, 11 Oct 2026 00:37:02 -0400 Subject: [PATCH 1/3] Retain compositor resources when GPU completion is unconfirmed --- app/src/main/cpp/waylandcomp/src/vk_present.c | 53 ++++- docs/development/gpu-pending-completion.md | 37 ++++ tools/tests/test_vk_pending_completion.py | 183 ++++++++++++++++++ 3 files changed, 264 insertions(+), 9 deletions(-) create mode 100644 docs/development/gpu-pending-completion.md create mode 100644 tools/tests/test_vk_pending_completion.py diff --git a/app/src/main/cpp/waylandcomp/src/vk_present.c b/app/src/main/cpp/waylandcomp/src/vk_present.c index 4551c4806..24c037c5d 100644 --- a/app/src/main/cpp/waylandcomp/src/vk_present.c +++ b/app/src/main/cpp/waylandcomp/src/vk_present.c @@ -39,7 +39,8 @@ struct vkp_image { static ANativeWindow *g_window; /* the window frames go to; compositor thread only */ static char *g_driver_path, *g_library_name, *g_native_lib_dir; -static int g_dev_state; /* 0 = not yet, 1 = ok, -1 = failed, -2 = lost (VK_ERROR_DEVICE_LOST) */ +static int g_dev_state; /* 0 = not yet, 1 = ok, -1 = failed, -2 = lost, -3 = completion unknown */ +static void device_lost(const char *where); static VkInstance g_inst; static VkPhysicalDevice g_pd; static VkDevice g_dev; @@ -112,11 +113,24 @@ static void perf_add(int64_t *sum, int64_t *max, unsigned *count, int64_t d) { if (d > *max) *max = d; (*count)++; } +/* A timeout is not device loss and does not cancel a submit. Keep its images + * and memory alive until this compositor process exits. Resuming + * would reset a pending command buffer/fence or free memory still read by the GPU. */ +static void quarantine_pending_work(VkResult result, const char *where) { + if (g_dev_state != 1) return; + g_dev_state = -3; + g_base_black = 0; + LOGE("GPU completion unconfirmed (%d in %s): presenting stopped; pending resources retained; " + "restart DroidDeck", (int)result, where); +} + /* vkWaitForFences on the compositor's own work, timed for the perf line. */ static VkResult timed_wait(VkFence fence, uint64_t timeout) { int64_t t0 = perf_now(); VkResult r = g_vk.WaitForFences(g_dev, 1, &fence, VK_TRUE, timeout); perf_add(&g_perf.wait_ns, &g_perf.wait_max_ns, &g_perf.waits, perf_now() - t0); + if (r != VK_SUCCESS && r != VK_ERROR_DEVICE_LOST) + quarantine_pending_work(r, "fence wait"); return r; } @@ -236,7 +250,12 @@ static void reset_sync(void) { static void destroy_swapchain(void) { g_base_black = 0; /* a new swapchain starts with nothing on it */ if (g_dev_state != 1) return; - g_vk.DeviceWaitIdle(g_dev); + VkResult idle = g_vk.DeviceWaitIdle(g_dev); + if (idle != VK_SUCCESS) { + if (idle == VK_ERROR_DEVICE_LOST) device_lost("swapchain idle"); + else quarantine_pending_work(idle, "swapchain idle"); + return; + } reset_sync(); for (uint32_t i = 0; i < g_nimg; i++) blendp_forget_image(g_images[i]); if (g_swapchain) g_vk.DestroySwapchainKHR(g_dev, g_swapchain, NULL); @@ -546,6 +565,7 @@ static int dev_init(void) { g_dev_state = 1; reset_sync(); init_black_image(); + if (g_dev_state != 1) return -1; vkp_framegen_device_ready(g_dev, g_queue, g_qfam, fg_features != NULL); return 0; } @@ -557,7 +577,7 @@ static int swap_init_locked(void); static int swap_init(void) { struct timespec ts; int64_t now; - if (!g_window) return -1; + if (!g_window || g_dev_state != 1) return -1; clock_gettime(CLOCK_MONOTONIC, &ts); now = (int64_t)ts.tv_sec * 1000000000LL + ts.tv_nsec; if (now < g_swap_retry_at_ns) return -1; @@ -842,10 +862,15 @@ static void init_black_image(void) { g_vk.EndCommandBuffer(cmd); VkSubmitInfo si = {.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO, .commandBufferCount = 1, .pCommandBuffers = &cmd}; g_vk.ResetFences(g_dev, 1, &g_fence); - if (g_vk.QueueSubmit(g_queue, 1, &si, g_fence) == VK_SUCCESS && - g_vk.WaitForFences(g_dev, 1, &g_fence, VK_TRUE, 1000000000ULL) == VK_SUCCESS) - g_black_state = 1; - else + VkResult qr = g_vk.QueueSubmit(g_queue, 1, &si, g_fence); + if (qr == VK_SUCCESS) { + VkResult fr = timed_wait(g_fence, 1000000000ULL); + if (fr == VK_SUCCESS) g_black_state = 1; + else if (fr == VK_ERROR_DEVICE_LOST) device_lost("black image"); + } else if (qr == VK_ERROR_DEVICE_LOST) { + device_lost("black image submit"); + } + if (g_black_state != 1) droiddeck_log("gpu", "letterbox black image unavailable: the whole output is cleared every copy-path frame"); } @@ -1100,6 +1125,7 @@ int vkp_image_height(const struct vkp_image *img) { return img ? img->h : 0; } void vkp_image_destroy(struct vkp_image *img) { if (!img) return; + if (g_dev_state == -3) return; /* completion unknown: retain GPU ownership */ if (g_dev) { blendp_forget_image(img->image); if (img->map) g_vk.UnmapMemory(g_dev, img->mem); @@ -1550,6 +1576,7 @@ static int render_impl(int scene_w, int scene_h, const struct vkp_draw *draws, i : "no HDR frames through the screen swapchain any more: rebuilding it as SDR"); destroy_swapchain(); } + if (g_dev_state != 1) return -1; g_swap_hdr_want = want_hdr; uint32_t img = 0; @@ -1798,11 +1825,17 @@ static int render_impl(int scene_w, int scene_h, const struct vkp_draw *draws, i /* Ended early: an empty submit carrying the fence is ordered after everything queued * above, so the wait below still means "this frame's work is done". */ VkSubmitInfo si = {.sType = VK_STRUCTURE_TYPE_SUBMIT_INFO}; - g_vk.QueueSubmit(g_queue, 1, &si, g_fence); + VkResult marker = g_vk.QueueSubmit(g_queue, 1, &si, g_fence); + if (marker != VK_SUCCESS) { + if (marker == VK_ERROR_DEVICE_LOST) device_lost("frame completion submit"); + else quarantine_pending_work(marker, "frame completion submit"); + return -1; /* no fence was submitted; never wait on its unsignalled state */ + } } VkResult fr = timed_wait(g_fence, UINT64_MAX); - if (fg) vkp_framegen_presented(presented_gen); if (fr == VK_ERROR_DEVICE_LOST || pr == VK_ERROR_DEVICE_LOST) { device_lost("present"); return -1; } + if (fr != VK_SUCCESS) return -1; + if (fg) vkp_framegen_presented(presented_gen); if (pr == VK_ERROR_OUT_OF_DATE_KHR || pr == VK_ERROR_SURFACE_LOST_KHR) { /* The surface changed or went away under this frame: rebuild before the next one. */ droiddeck_log("gpu", "screen surface %s on present: rebuilding the swapchain", vk_result_name(pr)); @@ -1876,6 +1909,7 @@ void vkp_signal_first_frame(void) { int vkp_update_map(int scene_w, int scene_h) { if (g_dev_state == -2 || dev_init() != 0 || !g_window || scene_w <= 0 || scene_h <= 0) return -1; check_surface_changed(); /* layer mode may present nothing on the base surface for a long time */ + if (g_dev_state != 1) return -1; if (!g_swapchain && swap_init() != 0) return -1; update_map(scene_w, scene_h); return g_map.valid ? 0 : -1; @@ -2114,6 +2148,7 @@ static int pass_begin_impl(int scene_w, int scene_h, const struct vkp_draw *draw if (g_pass.active) vkp_pass_abort(); if (g_dev_state == -2 || dev_init() != 0 || !g_window || scene_w <= 0 || scene_h <= 0) return -1; check_surface_changed(); /* as in render_impl: the effects pass maps onto the output's size */ + if (g_dev_state != 1) return -1; if (!g_swapchain && swap_init() != 0) return -1; /* the mapping is in output pixels */ update_map(scene_w, scene_h); if (!g_map.valid) return -1; diff --git a/docs/development/gpu-pending-completion.md b/docs/development/gpu-pending-completion.md new file mode 100644 index 000000000..16304d038 --- /dev/null +++ b/docs/development/gpu-pending-completion.md @@ -0,0 +1,37 @@ +# Compositor work whose completion is unknown + +A timed-out fence wait does not cancel submitted GPU work. Continuing after +that result can reset a command buffer or fence still in use, or destroy an +image and its memory while the GPU may still read them. Previously the finite +wait paths returned without putting the compositor into a terminal state, and +swapchain teardown ignored the result of `vkDeviceWaitIdle`. + +An unsuccessful completion check other than real `VK_ERROR_DEVICE_LOST` now +puts the compositor in a distinct completion-unknown state. It stops subsequent +presentation and retains potentially pending image/memory and swapchain +resources until the DroidDeck app process exits. Device loss continues through +the existing device-loss path. This is deliberate retention rather than an +automatic recovery; restarting a guest session cannot reset the in-process GPU +state, so the diagnostic asks for an app restart. + +Initialization checks the black-image submit and fence result. An unsuccessful +empty completion-marker submit no longer causes a wait on an unsignalled fence. +Surface-rebuild and effect-map paths stop when teardown cannot establish idle. +The normal frame fence still has its existing infinite timeout; this change is +not a general GPU hang watchdog. + +## Regression coverage + +`python3 -m unittest discover -s tools/tests -p test_vk_pending_completion.py -v` +compiles the production wait/destructor and swapchain teardown functions with a +fake Vulkan dispatch table. The two cases cover success, timeout, host-memory +errors, genuine device loss, resource retention and suppressed repeated teardown. +No real GPU fault is injected. The harness matches main's Vulkan-image resources; +it does not assume the experimental PowerVR branch's additional AHB fields. + +The baseline production fence/destructor compiles but fails the timeout-state +oracle; the corrected functions pass both cases with `-Wall -Wextra -Werror`. +The change was also exercised in an independently integrated Pixel PowerVR Lab, +but its short game observations do not establish the cause of the earlier GNOC +reboot or sustained device stability. Main's native CI and APK build remain +separate validation. diff --git a/tools/tests/test_vk_pending_completion.py b/tools/tests/test_vk_pending_completion.py new file mode 100644 index 000000000..5563f5cfa --- /dev/null +++ b/tools/tests/test_vk_pending_completion.py @@ -0,0 +1,183 @@ +"""Fault-inject the production fence wait and imported-image destructor. + +No GPU is wedged: a fake dispatch table returns the same result a timed-out +submit would return. The compiled functions come from vk_present.c itself. +""" +import os +from pathlib import Path +import re +import shutil +import subprocess +import tempfile +import unittest + +SOURCE = Path(__file__).resolve().parents[2] / 'app/src/main/cpp/waylandcomp/src/vk_present.c' + + +def function(text, name): + match = re.search(r'^(?:static )?[^\n;]+\b' + name + r'\([^;]*?\)\s*\{', text, re.M) + if not match: + raise AssertionError('Production function not found: ' + name) + start = match.start() + pos = text.index('{', match.start()) + depth = 1 + end = pos + 1 + while depth: + depth += (text[end] == '{') - (text[end] == '}') + end += 1 + return text[start:end] + + +HARNESS = r''' +#include +#include +#include +#include +typedef int VkResult; +typedef uintptr_t VkFence; +#define VK_TRUE 1 +#define VK_SUCCESS 0 +#define VK_TIMEOUT 2 +#define VK_ERROR_DEVICE_LOST -4 +#define VK_ERROR_OUT_OF_HOST_MEMORY -1 +static int g_dev_state, g_base_black; +static uintptr_t g_dev = 1; +static struct { int64_t wait_ns, wait_max_ns; unsigned waits; } g_perf; +static VkResult answer; +static unsigned destroyed, unmapped, freed_memory, forgotten, errors; +static int64_t perf_now(void) { return 10; } +static void perf_add(int64_t *s, int64_t *m, unsigned *c, int64_t d) { *s+=d; *m=d; ++*c; } +static void __attribute__((unused)) droiddeck_log(const char *tag, const char *fmt, ...) { (void)tag; (void)fmt; ++errors; } +#define LOGE(...) droiddeck_log("error", __VA_ARGS__) +static VkResult wait_fn(uintptr_t dev, unsigned n, const VkFence *f, int all, uint64_t timeout) { + (void)dev; (void)n; (void)f; (void)all; (void)timeout; return answer; +} +static void unmap_fn(uintptr_t dev, uintptr_t mem) { (void)dev; (void)mem; ++unmapped; } +static void destroy_fn(uintptr_t dev, uintptr_t img, void *allocator) { (void)dev; (void)img; (void)allocator; ++destroyed; } +static void free_fn(uintptr_t dev, uintptr_t mem, void *allocator) { (void)dev; (void)mem; (void)allocator; ++freed_memory; } +static struct { + VkResult (*WaitForFences)(uintptr_t, unsigned, const VkFence *, int, uint64_t); + void (*UnmapMemory)(uintptr_t, uintptr_t); + void (*DestroyImage)(uintptr_t, uintptr_t, void *); + void (*FreeMemory)(uintptr_t, uintptr_t, void *); +} g_vk = {wait_fn, unmap_fn, destroy_fn, free_fn}; +struct vkp_image { uintptr_t image, mem; void *map; }; +static void blendp_forget_image(uintptr_t image) { (void)image; ++forgotten; } +/* PRODUCTION_FUNCTIONS */ +static struct vkp_image *image(void) { + struct vkp_image *p = calloc(1, sizeof(*p)); assert(p); + p->image=1; p->mem=2; p->map=p; return p; +} +int main(void) { + g_dev_state=1; answer=VK_SUCCESS; + assert(timed_wait(1, 1000)==VK_SUCCESS); + assert(g_dev_state==1); + vkp_image_destroy(image()); + assert(destroyed==1 && unmapped==1 && freed_memory==1 && forgotten==1); + + g_dev_state=1; g_base_black=1; answer=VK_TIMEOUT; + assert(timed_wait(1, 1000)==VK_TIMEOUT); + assert(g_dev_state!=1 && g_dev_state!=-2); /* pending work is not DEVICE_LOST */ + struct vkp_image *pending=image(); + vkp_image_destroy(pending); + assert(destroyed==1 && unmapped==1 && freed_memory==1 && forgotten==1); + unsigned first_errors=errors; + assert(first_errors>0); + assert(timed_wait(1, 1000)==VK_TIMEOUT); + assert(errors==first_errors); /* one terminal diagnostic, no repeated recovery */ + free(pending); /* fake GPU exists only in this test */ + + g_dev_state=1; answer=VK_ERROR_OUT_OF_HOST_MEMORY; + assert(timed_wait(1, 1000)==VK_ERROR_OUT_OF_HOST_MEMORY); + assert(g_dev_state!=1 && g_dev_state!=-2); + pending=image(); vkp_image_destroy(pending); + assert(destroyed==1 && unmapped==1 && freed_memory==1 && forgotten==1); + free(pending); + + g_dev_state=1; answer=VK_ERROR_DEVICE_LOST; + assert(timed_wait(1, 1000)==VK_ERROR_DEVICE_LOST); + assert(g_dev_state==1); /* existing caller performs genuine device-loss cleanup */ + return 0; +} +''' + +SWAPCHAIN_HARNESS = r''' +#include +#include +#include +typedef int VkResult; +#define VK_SUCCESS 0 +#define VK_ERROR_DEVICE_LOST -4 +#define VK_ERROR_OUT_OF_HOST_MEMORY -1 +#define VK_NULL_HANDLE 0 +static int g_dev_state, g_base_black, g_swap_md_identity; +static uintptr_t g_dev=1, g_inst=1, g_swapchain, g_surface, *g_images; +static uint32_t g_nimg; +static unsigned resets, forgotten, destroyed, errors, lost; +static VkResult answer; +#define LOGE(...) (++errors) +static VkResult idle_fn(uintptr_t dev) { (void)dev; return answer; } +static void destroy_fn(uintptr_t dev, uintptr_t obj, void *alloc) { + (void)dev; (void)obj; (void)alloc; ++destroyed; +} +static struct { + VkResult (*DeviceWaitIdle)(uintptr_t); + void (*DestroySwapchainKHR)(uintptr_t, uintptr_t, void *); + void (*DestroySurfaceKHR)(uintptr_t, uintptr_t, void *); +} g_vk={idle_fn, destroy_fn, destroy_fn}; +static void reset_sync(void) { ++resets; } +static void blendp_forget_image(uintptr_t img) { (void)img; ++forgotten; } +static void device_lost(const char *where) { (void)where; ++lost; g_dev_state=-2; } +/* PRODUCTION_FUNCTIONS */ +static void setup(void) { + g_dev_state=1; g_base_black=1; g_swap_md_identity=1; + g_swapchain=2; g_surface=3; g_nimg=1; + g_images=calloc(1,sizeof(*g_images)); assert(g_images); *g_images=4; +} +int main(void) { + setup(); answer=VK_SUCCESS; destroy_swapchain(); + assert(resets==1 && destroyed==2 && forgotten==1); + assert(!g_swapchain && !g_surface && !g_images && !g_nimg && !g_swap_md_identity); + setup(); answer=VK_ERROR_OUT_OF_HOST_MEMORY; destroy_swapchain(); + assert(g_dev_state==-3 && errors==1); + assert(resets==1 && destroyed==2 && forgotten==1); + assert(g_swapchain && g_surface && g_images && g_nimg); + destroy_swapchain(); + assert(errors==1 && resets==1 && destroyed==2); + free(g_images); /* fake GPU resources only */ + setup(); answer=VK_ERROR_DEVICE_LOST; destroy_swapchain(); + assert(lost==1 && g_dev_state==-2); + assert(resets==1 && destroyed==2 && forgotten==1); + free(g_images); + return 0; +} +''' + + +class VkPendingCompletionTest(unittest.TestCase): + def compile_and_run(self, harness, names): + cc = shutil.which(os.environ.get('CC', 'cc')) + self.assertIsNotNone(cc, 'A C compiler is required') + text = SOURCE.read_text() + production = '\n'.join(function(text, n) for n in names) + with tempfile.TemporaryDirectory(prefix='droiddeck-pending-') as tmp: + source=Path(tmp)/'probe.c'; binary=Path(tmp)/'probe' + source.write_text(harness.replace('/* PRODUCTION_FUNCTIONS */', production)) + built=subprocess.run([cc,'-std=c11','-Wall','-Wextra','-Werror','-UNDEBUG',str(source),'-o',str(binary)],capture_output=True,text=True) + self.assertEqual(built.returncode,0,built.stdout+built.stderr) + ran=subprocess.run([str(binary)],capture_output=True,text=True,timeout=10) + self.assertEqual(ran.returncode,0,ran.stdout+ran.stderr) + + def test_unknown_completion_retains_gpu_resources(self): + self.compile_and_run(HARNESS, ['quarantine_pending_work', 'timed_wait', 'vkp_image_destroy']) + + def test_failed_device_idle_does_not_destroy_swapchain(self): + # LOGE's arguments must be evaluated as real production logging does. + harness = SWAPCHAIN_HARNESS.replace('#define LOGE(...) (++errors)', + 'static void log_error(const char *fmt, int result, const char *where) { ' + '(void)fmt; (void)result; (void)where; ++errors; }\n#define LOGE(...) log_error(__VA_ARGS__)') + self.compile_and_run(harness, ['quarantine_pending_work', 'destroy_swapchain']) + + +if __name__=='__main__': + unittest.main() From 37b775a9b6792ead5d6a379315ddf6770265c64c Mon Sep 17 00:00:00 2001 From: Leask Wong Date: Sun, 11 Oct 2026 01:18:18 -0400 Subject: [PATCH 2/3] Stop quarantined GPU callers before retries and mapped writes --- app/src/main/cpp/waylandcomp/src/vk_present.c | 25 +- docs/development/gpu-pending-completion.md | 27 ++ tools/tests/test_vk_pending_completion.py | 3 +- tools/tests/test_vk_terminal_callers.py | 115 +++++++++ tools/tests/vk_terminal_callers_test.c | 242 ++++++++++++++++++ 5 files changed, 404 insertions(+), 8 deletions(-) create mode 100644 tools/tests/test_vk_terminal_callers.py create mode 100644 tools/tests/vk_terminal_callers_test.c diff --git a/app/src/main/cpp/waylandcomp/src/vk_present.c b/app/src/main/cpp/waylandcomp/src/vk_present.c index 24c037c5d..dfd3e9f5d 100644 --- a/app/src/main/cpp/waylandcomp/src/vk_present.c +++ b/app/src/main/cpp/waylandcomp/src/vk_present.c @@ -126,6 +126,8 @@ static void quarantine_pending_work(VkResult result, const char *where) { /* vkWaitForFences on the compositor's own work, timed for the perf line. */ static VkResult timed_wait(VkFence fence, uint64_t timeout) { + if (g_dev_state != 1) + return g_dev_state == -2 ? VK_ERROR_DEVICE_LOST : VK_ERROR_UNKNOWN; int64_t t0 = perf_now(); VkResult r = g_vk.WaitForFences(g_dev, 1, &fence, VK_TRUE, timeout); perf_add(&g_perf.wait_ns, &g_perf.wait_max_ns, &g_perf.waits, perf_now() - t0); @@ -1109,10 +1111,10 @@ struct vkp_image *vkp_image_create_shm(int w, int h) { return img; } -/* Renders are synchronous (we wait for the frame's fence), so writing between frames - * never races the GPU. */ +/* Between successfully completed frames the GPU is idle. After an unconfirmed + * wait, retain the mapped bytes too: the previous submit may still read them. */ void vkp_image_upload_shm(struct vkp_image *img, const void *data, int stride) { - if (!img || !img->map || !data) return; + if (g_dev_state != 1 || !img || !img->map || !data) return; size_t rowbytes = (size_t)img->w * 4; if ((size_t)stride < rowbytes) rowbytes = (size_t)stride; for (int y = 0; y < img->h; y++) @@ -1415,6 +1417,7 @@ static void record_screen_blit(VkCommandBuffer cmd, VkImage src, int src_w, int * so they never rebuild; they fail and the frame ends early. 0 on success. */ static int acquire_image(int k, int first, uint32_t *img, VkResult *ar_out) { for (int attempt = 0; ; attempt++) { + if (g_dev_state != 1) return -1; if (!g_swapchain && (!first || swap_init() != 0)) return -1; /* A bounded wait: a surface that went away without telling us must not park the * compositor thread forever (clients are paced from this thread). */ @@ -1426,7 +1429,7 @@ static int acquire_image(int k, int first, uint32_t *img, VkResult *ar_out) { if (ar == VK_ERROR_OUT_OF_DATE_KHR || ar == VK_ERROR_SURFACE_LOST_KHR) { droiddeck_log("gpu", "screen surface %s on acquire: rebuilding the swapchain", vk_result_name(ar)); destroy_swapchain(); - if (first && attempt == 0) continue; + if (first && attempt == 0 && g_dev_state == 1) continue; return -1; } droiddeck_log("error", "present: acquire %d failed (%s %d)", k, vk_result_name(ar), (int)ar); @@ -1554,6 +1557,7 @@ static void send_hdr_metadata(const struct droiddeck_color *c) { static int render_impl(int scene_w, int scene_h, const struct vkp_draw *draws, int n, const struct vkp_hdr_frame *hf, int *how) { if (how) *how = 0; + if (g_dev_state < 0) return -1; vkp_apply_window_request(); check_surface_changed(); /* a rotation or a fold the rebind did not cover: rebuild at the new size */ if (g_dev_state == -2) return -1; @@ -1787,6 +1791,10 @@ static int render_impl(int scene_w, int scene_h, const struct vkp_draw *draws, i if (k > 0) { VkResult ark; if (acquire_image(k, 0, &img, &ark) != 0) { + /* Teardown may have retained this frame's pending work, or + * acquire may have reported real device loss. Neither permits + * a completion-marker submit or another fence wait. */ + if (g_dev_state != 1) return -1; droiddeck_log("framegen", "present %d/%d: no swapchain image; the frame ends early", k + 1, presents); pr = VK_ERROR_OUT_OF_DATE_KHR; break; @@ -1819,7 +1827,8 @@ static int render_impl(int scene_w, int scene_h, const struct vkp_draw *draws, i pr = g_vk.QueuePresentKHR(g_queue, &pi); perf_add(&g_perf.present_ns, &g_perf.present_max_ns, &g_perf.presents, perf_now() - t_pres); if (k < ngen) presented_gen++; - if (pr == VK_ERROR_OUT_OF_DATE_KHR || pr == VK_ERROR_SURFACE_LOST_KHR || pr == VK_ERROR_DEVICE_LOST) break; + if (pr == VK_ERROR_DEVICE_LOST) { device_lost("present"); return -1; } + if (pr == VK_ERROR_OUT_OF_DATE_KHR || pr == VK_ERROR_SURFACE_LOST_KHR) break; } if (!fence_submitted) { /* Ended early: an empty submit carrying the fence is ordered after everything queued @@ -2244,8 +2253,9 @@ static int pass_begin_impl(int scene_w, int scene_h, const struct vkp_draw *draw int vkp_pass_copy_to(struct vkp_image *dst, int wait_fd) { if (!g_pass.active) { if (wait_fd >= 0) close(wait_fd); return -1; } g_pass.active = 0; + if (g_dev_state != 1) { if (wait_fd >= 0) close(wait_fd); return -1; } VkCommandBuffer cmd = g_cmds[0]; - if (!dst || !dst->blit_dst || g_dev_state == -2) { + if (!dst || !dst->blit_dst) { if (wait_fd >= 0) close(wait_fd); g_vk.EndCommandBuffer(cmd); return -1; @@ -2300,5 +2310,6 @@ int vkp_pass_copy_to(struct vkp_image *dst, int wait_fd) { void vkp_pass_abort(void) { if (!g_pass.active) return; g_pass.active = 0; - g_vk.EndCommandBuffer(g_cmds[0]); /* never submitted; the buffer is reset before its next use */ + if (g_dev_state == 1) + g_vk.EndCommandBuffer(g_cmds[0]); /* never submitted; reset before its next use */ } diff --git a/docs/development/gpu-pending-completion.md b/docs/development/gpu-pending-completion.md index 16304d038..3beb4d1dd 100644 --- a/docs/development/gpu-pending-completion.md +++ b/docs/development/gpu-pending-completion.md @@ -17,6 +17,14 @@ state, so the diagnostic asks for an app restart. Initialization checks the black-image submit and fence result. An unsuccessful empty completion-marker submit no longer causes a wait on an unsignalled fence. Surface-rebuild and effect-map paths stop when teardown cannot establish idle. +Acquire retries also stop: retaining the old swapchain is not permission to +acquire from it again. A later frame-generation acquire that ends in a terminal +state returns before the completion marker, fence wait or any further recording. +Real device loss on present is handled before attempting that marker, too. +Mapped SHM bytes are retained unchanged, including same-size `wl_shm` and +single-pixel commits; keeping only the allocation would still race a GPU read. +Off-screen pass copy/abort and the internal fence wait reject terminal states +without further Vulkan calls. Release fds are still consumed on rejected copies. The normal frame fence still has its existing infinite timeout; this change is not a general GPU hang watchdog. @@ -29,6 +37,25 @@ errors, genuine device loss, resource retention and suppressed repeated teardown No real GPU fault is injected. The harness matches main's Vulkan-image resources; it does not assume the experimental PowerVR branch's additional AHB fields. +`python3 -m unittest discover -s tools/tests -p test_vk_terminal_callers.py -v` +additionally compiles the **entire production `vk_present.c`**, with fake Vulkan +dispatch callbacks and stub external modules. The compositor's `take_shm` and +`take_pixel` callers are inserted verbatim, not rewritten. It needs the Android +NDK's Vulkan-Headers (`ANDROID_HOME`/`ANDROID_SDK_ROOT`/`NDK`), a host Vulkan +development package, or `DROIDDECK_VULKAN_INCLUDE`; absent headers are explicitly +reported as a skip. No Android runtime or GPU is required. + +The 12 scenarios include first/later OUT_OF_DATE and SURFACE_LOST followed by an +idle failure, a finite blit timeout followed by same-size SHM/single-pixel +uploads, normal presentation/frame generation, a healthy later acquire timeout +whose earlier work still needs draining, real acquire/present device loss, and +terminal off-screen pass entry. Every work-issuing fake callback asserts the +device is live. After quarantine, the tests verify retained handles and unchanged +mapped bytes, and probe the public render/blit/readback/import/pass entry points +for suppressed acquire/submit/wait/reset and destruction. Consumed fds are checked. +Against PR #560's `68dc1c45` the same harness compiles, but nine scenarios fail +(the three healthy controls pass); the corrected production unit passes all 12. + The baseline production fence/destructor compiles but fails the timeout-state oracle; the corrected functions pass both cases with `-Wall -Wextra -Werror`. The change was also exercised in an independently integrated Pixel PowerVR Lab, diff --git a/tools/tests/test_vk_pending_completion.py b/tools/tests/test_vk_pending_completion.py index 5563f5cfa..5f69c820d 100644 --- a/tools/tests/test_vk_pending_completion.py +++ b/tools/tests/test_vk_pending_completion.py @@ -40,6 +40,7 @@ typedef uintptr_t VkFence; #define VK_TIMEOUT 2 #define VK_ERROR_DEVICE_LOST -4 #define VK_ERROR_OUT_OF_HOST_MEMORY -1 +#define VK_ERROR_UNKNOWN -13 static int g_dev_state, g_base_black; static uintptr_t g_dev = 1; static struct { int64_t wait_ns, wait_max_ns; unsigned waits; } g_perf; @@ -83,7 +84,7 @@ int main(void) { assert(destroyed==1 && unmapped==1 && freed_memory==1 && forgotten==1); unsigned first_errors=errors; assert(first_errors>0); - assert(timed_wait(1, 1000)==VK_TIMEOUT); + assert(timed_wait(1, 1000)==VK_ERROR_UNKNOWN); assert(errors==first_errors); /* one terminal diagnostic, no repeated recovery */ free(pending); /* fake GPU exists only in this test */ diff --git a/tools/tests/test_vk_terminal_callers.py b/tools/tests/test_vk_terminal_callers.py new file mode 100644 index 000000000..caacb47a0 --- /dev/null +++ b/tools/tests/test_vk_terminal_callers.py @@ -0,0 +1,115 @@ +"""Fault-inject complete production render/blit/upload callers, without a GPU. + +Uses Vulkan-Headers from the Android NDK (as installed by APK CI), or the host +Vulkan development package. Only Android platform declarations and the external +effect/frame-generation modules are mocked; vk_present.c is compiled unmodified. +""" +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + +from test_vk_pending_completion import function + +ROOT = Path(__file__).resolve().parents[2] +SOURCE = ROOT / 'app/src/main/cpp/waylandcomp/src/vk_present.c' + + +def vulkan_include(): + candidates = [Path(p) for p in [os.environ.get('DROIDDECK_VULKAN_INCLUDE', ''), + '/usr/include', '/usr/local/include'] if p] + for name in ('ANDROID_HOME', 'ANDROID_SDK_ROOT', 'ANDROID_NDK_HOME', 'NDK'): + if os.environ.get(name): + root = Path(os.environ[name]) + candidates += sorted(root.glob('ndk/*/toolchains/llvm/prebuilt/*/sysroot/usr/include'), reverse=True) + candidates += list(root.glob('toolchains/llvm/prebuilt/*/sysroot/usr/include')) + return next((p for p in candidates if (p / 'vulkan/vulkan.h').is_file()), None) + + +class VkTerminalCallersTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + headers = vulkan_include() + if headers is None: + raise unittest.SkipTest('NDK or host Vulkan-Headers required for full production-C caller tests') + cc = shutil.which(os.environ.get('CC', 'cc')) + if not cc: + raise AssertionError('C compiler required') + cls.tmp = tempfile.TemporaryDirectory(prefix='droiddeck-callers-') + cls.addClassCleanup(cls.tmp.cleanup) + tmp = Path(cls.tmp.name) + (tmp / 'android').mkdir() + (tmp / 'android/native_window.h').write_text(''' +#pragma once +#include +typedef struct ANativeWindow ANativeWindow; +int32_t ANativeWindow_getWidth(ANativeWindow *); +int32_t ANativeWindow_getHeight(ANativeWindow *); +void ANativeWindow_release(ANativeWindow *); +''') + (tmp / 'android/hardware_buffer.h').write_text('#pragma once\ntypedef struct AHardwareBuffer AHardwareBuffer;\n') + (tmp / 'android/log.h').write_text(''' +#pragma once +#define ANDROID_LOG_INFO 4 +int __android_log_print(int, const char *, const char *, ...); +''') + (tmp / 'wayland-server.h').write_text('#pragma once\nstruct wl_display; struct wl_resource; struct wl_client;\n') + compositor = (SOURCE.parent / 'compositor.c').read_text() + callers = '\n'.join(function(compositor, n) for n in ('take_shm', 'take_pixel')) + harness = (Path(__file__).with_name('vk_terminal_callers_test.c')).read_text() + probe = tmp / 'probe.c' + probe.write_text(harness.replace('/* COMPOSITOR_CALLERS */', callers)) + cls.binary = tmp / 'probe' + built = subprocess.run([cc, '-std=c11', '-Wall', '-Wextra', '-Werror', + '-Wno-unused-parameter', '-Wno-missing-field-initializers', '-UNDEBUG', + '-I', str(tmp), '-idirafter', str(headers), '-I', str(SOURCE.parent), + str(probe), '-pthread', '-o', str(cls.binary)], + capture_output=True, text=True) + if built.returncode: + raise AssertionError(built.stdout + built.stderr) + + def run_scenario(self, name): + ran = subprocess.run([str(self.binary), name], capture_output=True, text=True, timeout=10) + self.assertEqual(ran.returncode, 0, ran.stdout + ran.stderr) + + def test_first_acquire_idle_failure(self): + self.run_scenario('first-out-of-date') + + def test_first_surface_lost_idle_failure(self): + self.run_scenario('first-surface-lost') + + def test_later_framegen_acquire_idle_failure(self): + self.run_scenario('later-out-of-date') + + def test_later_framegen_surface_lost_idle_failure(self): + self.run_scenario('later-surface-lost') + + def test_finite_blit_timeout_same_size_shm_upload(self): + self.run_scenario('shm-timeout') + + def test_finite_blit_timeout_same_size_pixel_upload(self): + self.run_scenario('pixel-timeout') + + def test_normal_present(self): + self.run_scenario('normal') + + def test_normal_framegen_present(self): + self.run_scenario('framegen') + + def test_healthy_later_acquire_timeout_drains_work(self): + self.run_scenario('later-timeout') + + def test_real_device_lost_on_acquire(self): + self.run_scenario('acquire-device-lost') + + def test_real_device_lost_on_present(self): + self.run_scenario('present-device-lost') + + def test_pass_copy_and_abort_terminal_entry(self): + self.run_scenario('pass-terminal') + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/tests/vk_terminal_callers_test.c b/tools/tests/vk_terminal_callers_test.c new file mode 100644 index 000000000..e395df07d --- /dev/null +++ b/tools/tests/vk_terminal_callers_test.c @@ -0,0 +1,242 @@ +/* Compiled with the complete, unmodified production translation unit. The + * compositor's take_shm/take_pixel functions are inserted verbatim by Python. */ +#include "vk_present.c" +#include + +struct vk_api g_vk; +static unsigned operations, acquires, submits, markers, waits, idle_calls, resets; +static unsigned destroys, releases, lost_notifications, presented_notifications; +static VkResult idle_answer, fence_answer, acquire_answer, present_answer; +static unsigned fail_acquire_at; +static int generated; + +/* Every work-issuing callback is an oracle: no GPU operation after a terminal + * result. Object destruction is counted separately for genuine device loss. */ +static void live(void) { assert(g_dev_state == 1); ++operations; } +static VkResult acquire(VkDevice d, VkSwapchainKHR s, uint64_t t, VkSemaphore a, VkFence f, uint32_t *i) { + live(); ++acquires; *i = 0; + return acquires == fail_acquire_at ? acquire_answer : VK_SUCCESS; +} +static VkResult idle(VkDevice d) { live(); ++idle_calls; return idle_answer; } +static VkResult wait_fences(VkDevice d, uint32_t n, const VkFence *f, VkBool32 all, uint64_t t) { + live(); ++waits; return fence_answer; +} +static VkResult submit(VkQueue q, uint32_t n, const VkSubmitInfo *s, VkFence f) { + live(); ++submits; if (!s->commandBufferCount) ++markers; return VK_SUCCESS; +} +static VkResult present(VkQueue q, const VkPresentInfoKHR *p) { live(); return present_answer; } +static VkResult reset_cmd(VkCommandBuffer c, VkCommandBufferResetFlags f) { live(); ++resets; return VK_SUCCESS; } +static VkResult reset_fence(VkDevice d, uint32_t n, const VkFence *f) { live(); ++resets; return VK_SUCCESS; } +static VkResult begin_cmd(VkCommandBuffer c, const VkCommandBufferBeginInfo *b) { live(); return VK_SUCCESS; } +static VkResult end_cmd(VkCommandBuffer c) { live(); return VK_SUCCESS; } +static void barrier(VkCommandBuffer c, VkPipelineStageFlags s, VkPipelineStageFlags d, VkDependencyFlags f, + uint32_t nm, const VkMemoryBarrier *m, uint32_t nb, const VkBufferMemoryBarrier *b, + uint32_t ni, const VkImageMemoryBarrier *i) { live(); } +static void clear(VkCommandBuffer c, VkImage i, VkImageLayout l, const VkClearColorValue *v, + uint32_t n, const VkImageSubresourceRange *r) { live(); } +static void blit(VkCommandBuffer c, VkImage s, VkImageLayout sl, VkImage d, VkImageLayout dl, + uint32_t n, const VkImageBlit *b, VkFilter f) { live(); } +static void destroy_swap(VkDevice d, VkSwapchainKHR s, const VkAllocationCallbacks *a) { ++destroys; } +static void destroy_surface(VkInstance i, VkSurfaceKHR s, const VkAllocationCallbacks *a) { ++destroys; } +static void destroy_sem(VkDevice d, VkSemaphore s, const VkAllocationCallbacks *a) { live(); ++destroys; } +static VkResult create_sem(VkDevice d, const VkSemaphoreCreateInfo *c, const VkAllocationCallbacks *a, VkSemaphore *s) { + live(); *s = (VkSemaphore)(uintptr_t)1; return VK_SUCCESS; +} +static void destroy_image(VkDevice d, VkImage i, const VkAllocationCallbacks *a) { ++destroys; } +static void free_memory(VkDevice d, VkDeviceMemory m, const VkAllocationCallbacks *a) { ++destroys; } +static void unmap(VkDevice d, VkDeviceMemory m) { ++destroys; } + +/* External modules are not the subject of this fault injection. Their declared + * signatures remain checked by the production headers. */ +int __android_log_print(int p, const char *t, const char *f, ...) { return 0; } +void droiddeck_log(const char *t, const char *f, ...) {} +void droiddeck_on_first_frame(void) {} +int32_t ANativeWindow_getWidth(ANativeWindow *w) { return 64; } +int32_t ANativeWindow_getHeight(ANativeWindow *w) { return 64; } +void ANativeWindow_release(ANativeWindow *w) {} +void sc_layer_window_gone(void) {} +int vk_loader_open(const char *a, const char *b, const char *c) { assert(0); return -1; } +void vk_loader_load_instance(VkInstance i) { assert(0); } +void vk_loader_load_device(VkDevice d) { assert(0); } +PFN_vkGetInstanceProcAddr vk_loader_gipa(void) { return NULL; } +void vkp_effects_bind_device(VkDevice d, VkPhysicalDevice p, const VkPhysicalDeviceMemoryProperties *m) {} +int vkp_effects_active(void) { return 0; } +VkFilter vkp_effects_blit_filter(void) { return VK_FILTER_NEAREST; } +void vkp_effects_set_formats(VkFormat a, VkFormat b) {} +VkImage vkp_effects_run(VkCommandBuffer c, VkImage i, int w, int h, int ow, int oh, int *rw, int *rh) { + *rw = w; *rh = h; return i; +} +const void *vkp_framegen_device_features(VkInstance i, PFN_vkGetInstanceProcAddr p, VkPhysicalDevice d) { return NULL; } +void vkp_framegen_device_ready(VkDevice d, VkQueue q, uint32_t f, int e) {} +void vkp_framegen_device_lost(void) { ++lost_notifications; } +int vkp_framegen_active(void) { return generated != 0; } +int vkp_framegen_extra_images(void) { return generated; } +int vkp_framegen_format_ok(VkFormat f) { return 0; } +int vkp_framegen_run(VkCommandBuffer c, VkImage s, VkImageView v, int w, int h, + VkFormat f, VkImage images[VKP_FG_MAX_GENERATIONS]) { + for (int i = 0; i < generated; ++i) images[i] = s; + return generated; +} +void vkp_framegen_presented(int n) { ++presented_notifications; } +void hdrc_bind_device(VkDevice d, const VkPhysicalDeviceMemoryProperties *m) {} +void hdrc_forget_image(VkImage i) {} +int hdrc_encode(VkCommandBuffer c, VkImage a, VkImage b, VkFormat f, int w, int h, const struct hdrc_params *p) { return -1; } +void blendp_bind_device(VkDevice d) {} +void blendp_forget_image(VkImage i) {} +void blendp_begin_frame(void) {} +int blendp_draw(VkCommandBuffer c, VkImage s, VkFormat sf, int sw, int sh, int sg, + VkImage d, VkFormat df, int dw, int dh, const VkImageBlit *b) { return -1; } +int droiddeck_color_requested(void) { return 0; } +float droiddeck_color_sdr_white(void) { return 203.0f; } + +struct surface { + struct vkp_image *shm_img; + struct wl_resource *resource; + int buf_w, buf_h, buf_alpha, has_content; +}; +struct client_info { int asked_feedback, shm_frames; }; +struct wl_shm_buffer { int w, h, stride; const void *data; }; +#define WL_SHM_FORMAT_ARGB8888 0 +static unsigned g_stat_shm; +static int wl_shm_buffer_get_width(struct wl_shm_buffer *b) { return b->w; } +static int wl_shm_buffer_get_height(struct wl_shm_buffer *b) { return b->h; } +static int wl_shm_buffer_get_stride(struct wl_shm_buffer *b) { return b->stride; } +static int wl_shm_buffer_get_format(struct wl_shm_buffer *b) { return WL_SHM_FORMAT_ARGB8888; } +static const void *wl_shm_buffer_get_data(struct wl_shm_buffer *b) { return b->data; } +static void wl_shm_buffer_begin_access(struct wl_shm_buffer *b) {} +static void wl_shm_buffer_end_access(struct wl_shm_buffer *b) {} +static void wl_buffer_send_release(struct wl_resource *b) { ++releases; } +static struct wl_client *wl_resource_get_client(struct wl_resource *r) { return NULL; } +static struct client_info *client_info_of(struct wl_client *c) { return NULL; } +/* COMPOSITOR_CALLERS */ + +static void setup(int extra) { + g_vk = (struct vk_api){.AcquireNextImageKHR = acquire, .DeviceWaitIdle = idle, + .WaitForFences = wait_fences, .QueueSubmit = submit, .QueuePresentKHR = present, + .ResetCommandBuffer = reset_cmd, .ResetFences = reset_fence, .BeginCommandBuffer = begin_cmd, + .EndCommandBuffer = end_cmd, .CmdPipelineBarrier = barrier, .CmdClearColorImage = clear, + .CmdBlitImage = blit, .DestroySwapchainKHR = destroy_swap, .DestroySurfaceKHR = destroy_surface, + .DestroySemaphore = destroy_sem, .CreateSemaphore = create_sem, + .DestroyImage = destroy_image, .FreeMemory = free_memory, .UnmapMemory = unmap}; + idle_answer = fence_answer = acquire_answer = present_answer = VK_SUCCESS; + generated = g_swap_extra = extra; + g_dev_state = 1; + g_dev = (VkDevice)(uintptr_t)1; g_inst = (VkInstance)(uintptr_t)1; + g_window = (ANativeWindow *)(uintptr_t)1; g_queue = (VkQueue)(uintptr_t)1; + g_swapchain = (VkSwapchainKHR)(uintptr_t)2; g_surface = (VkSurfaceKHR)(uintptr_t)3; + g_images = calloc(1, sizeof(*g_images)); assert(g_images); + *g_images = (VkImage)(uintptr_t)4; g_nimg = 1; + g_extent = g_caps_extent = (VkExtent2D){64, 64}; + g_surface_checked_ns = INT64_MAX; /* no simulated geometry change */ + g_scene.img = (VkImage)(uintptr_t)5; g_scene.mem = (VkDeviceMemory)(uintptr_t)6; + g_scene.w = g_scene.h = 64; + for (int k = 0; k < MAX_PRESENTS; ++k) { + g_cmds[k] = (VkCommandBuffer)(uintptr_t)(10 + k); + g_acqs[k] = (VkSemaphore)(uintptr_t)(20 + k); + g_rnds[k] = (VkSemaphore)(uintptr_t)(30 + k); + } + g_fence = (VkFence)(uintptr_t)40; +} + +/* Probe the public GPU entry points after a terminal result. This includes an + * active off-screen pass and consumed release fds, not just the render guard. */ +static void assert_stopped(void) { + unsigned before = operations, before_destroys = destroys; + struct vkp_image src = {.image = (VkImage)(uintptr_t)7, .w = 1, .h = 1}; + struct vkp_image dst = {.image = (VkImage)(uintptr_t)8, .w = 1, .h = 1, .blit_dst = 1}; + uint32_t pixel = 0; uint64_t mod = 0; + assert(vkp_render(64, 64, NULL, 0) == -1); + assert(vkp_render_plain(64, 64) == -1); + assert(vkp_render_hdr(64, 64, NULL, 0, NULL, NULL) == -1); + assert(vkp_base_black(64, 64) == -1); + assert(vkp_update_map(64, 64) == -1); + assert(vkp_ready() == -1); + assert(vkp_image_create_shm(1, 1) == NULL); + assert(vkp_image_import_dmabuf(-1, 0, VKP_MOD_LINEAR, 1, 1, 4, 0, 0) == NULL); + assert(vkp_dmabuf_modifiers(0, &mod, 1) == 0); + assert(vkp_image_readback(&src, &pixel, 1) == -1); + assert(vkp_pass_begin(64, 64, NULL, 0, NULL, NULL) == -1); + int fd[2]; assert(pipe(fd) == 0); + assert(vkp_blit_image(&src, &dst, fd[0]) == -1); + errno = 0; assert(close(fd[0]) == -1 && errno == EBADF); close(fd[1]); + g_pass.active = 1; + assert(pipe(fd) == 0); + assert(vkp_pass_copy_to(&dst, fd[0]) == -1 && !g_pass.active); + errno = 0; assert(close(fd[0]) == -1 && errno == EBADF); close(fd[1]); + g_pass.active = 1; vkp_pass_abort(); assert(!g_pass.active); + assert(timed_wait(g_fence, UINT64_MAX) != VK_SUCCESS); + assert(operations == before && destroys == before_destroys); +} + +static void acquire_failure(int later, VkResult result) { + setup(later ? 1 : 0); + fail_acquire_at = later ? 2 : 1; acquire_answer = result; + idle_answer = VK_ERROR_OUT_OF_HOST_MEMORY; + VkImage *retained = g_images; + assert(vkp_render(64, 64, NULL, 0) == -1); + assert(g_dev_state == -3 && !vkp_device_lost()); + assert(acquires == fail_acquire_at && idle_calls == 1); + assert(submits == (unsigned)later && markers == 0 && waits == 0); + assert(destroys == 0 && g_images == retained && g_nimg == 1); + assert(g_swapchain && g_surface && g_fence && g_scene.img && g_scene.mem); + assert_stopped(); + free(g_images); /* test-owned fake resources only */ +} + +static void upload_timeout(int single_pixel) { + setup(0); + uint8_t bytes[48], saved[48], data[16]; + memset(bytes, 0xa5, sizeof(bytes)); memset(data, 0x3c, sizeof(data)); + struct vkp_image *img = calloc(1, sizeof(*img)); assert(img); + *img = (struct vkp_image){.image = (VkImage)(uintptr_t)7, .mem = (VkDeviceMemory)(uintptr_t)8, + .map = bytes, .offset = 3, .row_pitch = 16, + .w = single_pixel ? 1 : 2, .h = single_pixel ? 1 : 2}; + struct surface s = {.shm_img = img}; + struct wl_shm_buffer b = {.w = img->w, .h = img->h, .stride = img->w * 4, .data = data}; + if (single_pixel) take_pixel(&s, data, NULL); else take_shm(&s, &b, NULL); + assert(bytes[3] == 0x3c && bytes[0] == 0xa5 && releases == 1); + memcpy(saved, bytes, sizeof(saved)); + struct vkp_image dst = {.image = (VkImage)(uintptr_t)9, .blit_dst = 1, .w = img->w, .h = img->h}; + fence_answer = VK_TIMEOUT; + assert(vkp_blit_image(img, &dst, -1) == -1); + assert(g_dev_state == -3 && submits == 1 && waits == 1); + memset(data, 0x7e, sizeof(data)); + if (single_pixel) take_pixel(&s, data, NULL); else take_shm(&s, &b, NULL); + assert(s.shm_img == img && memcmp(saved, bytes, sizeof(saved)) == 0 && releases == 2); + vkp_image_destroy(img); + assert(destroys == 0 && img->map == bytes && img->image && img->mem); + assert_stopped(); + free(img); free(g_images); /* fake resources only, not production cleanup */ +} + +int main(int argc, char **argv) { + assert(argc == 2); + const char *name = argv[1]; + if (!strcmp(name, "first-out-of-date")) acquire_failure(0, VK_ERROR_OUT_OF_DATE_KHR); + else if (!strcmp(name, "first-surface-lost")) acquire_failure(0, VK_ERROR_SURFACE_LOST_KHR); + else if (!strcmp(name, "later-out-of-date")) acquire_failure(1, VK_ERROR_OUT_OF_DATE_KHR); + else if (!strcmp(name, "later-surface-lost")) acquire_failure(1, VK_ERROR_SURFACE_LOST_KHR); + else if (!strcmp(name, "shm-timeout")) upload_timeout(0); + else if (!strcmp(name, "pixel-timeout")) upload_timeout(1); + else if (!strcmp(name, "pass-terminal")) { setup(0); g_dev_state = -3; assert_stopped(); free(g_images); } + else if (!strcmp(name, "normal") || !strcmp(name, "framegen")) { + int extra = !strcmp(name, "framegen"); setup(extra); + assert(vkp_render(64, 64, NULL, 0) == 0 && g_dev_state == 1); + assert(acquires == (unsigned)(1 + extra) && submits == (unsigned)(1 + extra)); + assert(waits == 1 && markers == 0 && idle_calls == 0 && destroys == 0); + assert(presented_notifications == (unsigned)extra); free(g_images); + } else if (!strcmp(name, "later-timeout")) { + setup(1); fail_acquire_at = 2; acquire_answer = VK_TIMEOUT; + assert(vkp_render(64, 64, NULL, 0) == -1 && g_dev_state == 1); + assert(acquires == 2 && submits == 2 && markers == 1 && waits == 1); free(g_images); + } else if (!strcmp(name, "acquire-device-lost") || !strcmp(name, "present-device-lost")) { + setup(1); + if (!strcmp(name, "acquire-device-lost")) { fail_acquire_at = 2; acquire_answer = VK_ERROR_DEVICE_LOST; } + else present_answer = VK_ERROR_DEVICE_LOST; + assert(vkp_render(64, 64, NULL, 0) == -1 && vkp_device_lost()); + assert(g_dev_state == -2 && lost_notifications == 1 && markers == 0 && waits == 0); + assert(!g_images && !g_swapchain && !g_surface && destroys == 2); + assert_stopped(); + } else assert(0 && "unknown scenario"); + return 0; +} From 9fffb08f5aab41229f0ee2c13981208bf154982e Mon Sep 17 00:00:00 2001 From: Leask Wong Date: Sun, 11 Oct 2026 14:05:56 -0400 Subject: [PATCH 3/3] Retain external DMA-BUF ownership after GPU quarantine --- .../main/cpp/waylandcomp/src/ahb_swapchain.c | 4 + app/src/main/cpp/waylandcomp/src/compositor.c | 56 +++-- app/src/main/cpp/waylandcomp/src/vk_present.c | 1 + app/src/main/cpp/waylandcomp/src/vk_present.h | 4 + docs/development/gpu-pending-completion.md | 24 +++ tools/tests/test_vk_dmabuf_quarantine.py | 106 ++++++++++ tools/tests/test_vk_terminal_callers.py | 2 +- tools/tests/vk_dmabuf_quarantine_test.c | 199 ++++++++++++++++++ tools/tests/vk_terminal_callers_test.c | 7 +- 9 files changed, 384 insertions(+), 19 deletions(-) create mode 100644 tools/tests/test_vk_dmabuf_quarantine.py create mode 100644 tools/tests/vk_dmabuf_quarantine_test.c diff --git a/app/src/main/cpp/waylandcomp/src/ahb_swapchain.c b/app/src/main/cpp/waylandcomp/src/ahb_swapchain.c index 4a981f1ee..e567bea17 100644 --- a/app/src/main/cpp/waylandcomp/src/ahb_swapchain.c +++ b/app/src/main/cpp/waylandcomp/src/ahb_swapchain.c @@ -86,6 +86,9 @@ static struct ahb_buf *find_buf(uint64_t id) { } static void buf_free(struct ahb_buf *ab) { + /* A display release does not prove the compositor's Vulkan reads finished. + * Keep the AHB reference and its DMA-BUF record even if the client is gone. */ + if (vkp_completion_unknown()) return; if (ab->resource) { wl_list_remove(&ab->resource_destroy.link); ab->resource = NULL; } if (ab->fence_src) { wl_event_source_remove(ab->fence_src); ab->fence_src = NULL; } if (ab->fence_fd >= 0) { close(ab->fence_fd); ab->fence_fd = -1; } @@ -112,6 +115,7 @@ static void on_buffer_resource_destroyed(struct wl_listener *l, void *data) { /* ---- release: SurfaceFlinger is done with the buffer -> the game's next acquire waits, then wl_buffer.release */ static void send_deferred_release(struct ahb_buf *ab) { + if (vkp_completion_unknown()) return; if (ab->release_pending && ab->resource) droiddeck_release_buffer(ab->surface, ab->resource, ab->surface != NULL, ab->deferred_ns); ab->release_pending = 0; diff --git a/app/src/main/cpp/waylandcomp/src/compositor.c b/app/src/main/cpp/waylandcomp/src/compositor.c index 805eff9fa..e8a70fec8 100644 --- a/app/src/main/cpp/waylandcomp/src/compositor.c +++ b/app/src/main/cpp/waylandcomp/src/compositor.c @@ -377,6 +377,18 @@ static void perf_note_release(int64_t since_ns) { /* ------------------------------------------------------------------ buffer release pacing */ +/* Completion-unknown is terminal for externally owned DMA-BUF contents too. + * Keeping a VkImage alone cannot stop the client from overwriting its backing + * memory after wl_buffer.release. All paths (including cursors and idle roles) + * use this gate; copied SHM buffers retain their normal release behaviour. */ +static struct dmabuf_buffer *get_dmabuf(struct wl_resource *buffer); +static int buffer_release_blocked(struct wl_resource *buffer) { + return vkp_completion_unknown() && get_dmabuf(buffer) != NULL; +} +static void send_buffer_release(struct wl_resource *buffer) { + if (!buffer_release_blocked(buffer)) wl_buffer_send_release(buffer); +} + static void pending_release_free(struct pending_release *pr) { if (pr->surface && pr->surface->releases_pending > 0) pr->surface->releases_pending--; wl_list_remove(&pr->destroy.link); @@ -384,7 +396,8 @@ static void pending_release_free(struct pending_release *pr) { free(pr); } -/* A surface is going away: its queued releases still go out on time, they just stop counting. */ +/* A surface is going away: detach its accounting pointer, including queued + * DMA-BUF releases held in the completion-unknown state. */ static void pending_releases_forget_surface(struct surface *s) { struct pending_release *pr; wl_list_for_each(pr, &g_pending_releases, link) @@ -401,7 +414,8 @@ static void arm_release_timer(void) { int64_t earliest = 0; if (g_release_timer_fd < 0) return; wl_list_for_each(pr, &g_pending_releases, link) - if (!earliest || pr->at_ns < earliest) earliest = pr->at_ns; + if (!buffer_release_blocked(pr->buffer) && (!earliest || pr->at_ns < earliest)) + earliest = pr->at_ns; struct itimerspec its = {0}; if (earliest) { its.it_value.tv_sec = earliest / 1000000000LL; its.it_value.tv_nsec = earliest % 1000000000LL; } timerfd_settime(g_release_timer_fd, TFD_TIMER_ABSTIME, &its, NULL); @@ -413,8 +427,8 @@ static int on_release_timer(int fd, uint32_t mask, void *data) { int64_t now = now_ns() + 300000; /* anything due within 0.3 ms goes now */ if (read(fd, &expirations, sizeof(expirations)) < 0 && errno != EAGAIN) return 0; wl_list_for_each_safe(pr, tmp, &g_pending_releases, link) { - if (pr->at_ns <= now) { - wl_buffer_send_release(pr->buffer); + if (pr->at_ns <= now && !buffer_release_blocked(pr->buffer)) { + send_buffer_release(pr->buffer); perf_note_release(pr->since_ns); pending_release_free(pr); } @@ -435,6 +449,9 @@ static int on_release_timer(int fd, uint32_t mask, void *data) { * delivered (a swapchain rebuild destroys buffers with queued releases) or that a coarser earlier * limit had spaced out, and the game's first frames after that waited on empty slots. */ static void release_buffer(struct surface *s, struct wl_resource *buffer, int64_t since_ns) { + /* No allocation or immediate-release fallback is allowed for a quarantined + * DMA-BUF. Existing queued releases stay held; their timer is disarmed. */ + if (buffer_release_blocked(buffer)) return; const int limit = g_fps_limit; /* Without a limit of its own a game is paced to the screen. A swapchain that does not wait for * frame callbacks (MAILBOX, IMMEDIATE) is held back by nothing else here, so handing a replaced @@ -443,7 +460,7 @@ static void release_buffer(struct surface *s, struct wl_resource *buffer, int64_ * One release per refresh is the back-pressure X11 gives, and the screen shows the same frames * either way. */ const int64_t interval = limit > 0 ? 1000000000LL / limit : g_refresh_ns; - if (interval <= 0 || g_release_timer_fd < 0) { wl_buffer_send_release(buffer); perf_note_release(since_ns); return; } + if (interval <= 0 || g_release_timer_fd < 0) { send_buffer_release(buffer); perf_note_release(since_ns); return; } int64_t now = now_ns(); int64_t at = s->next_release_ns + interval; int64_t latest = now + interval * (int64_t)(s->releases_pending + 1); @@ -451,7 +468,7 @@ static void release_buffer(struct surface *s, struct wl_resource *buffer, int64_ if (at > latest) at = latest; s->next_release_ns = at; struct pending_release *pr = calloc(1, sizeof(*pr)); - if (!pr) { wl_buffer_send_release(buffer); perf_note_release(since_ns); return; } + if (!pr) { send_buffer_release(buffer); perf_note_release(since_ns); return; } pr->buffer = buffer; pr->surface = s; pr->at_ns = at; @@ -586,7 +603,10 @@ static void constraints_focus_entered(struct wl_resource *target, struct wl_clie /* ------------------------------------------------------------------ dmabuf buffers */ void dmabuf_buffer_unref(struct dmabuf_buffer *b) { - if (!b || --b->refs > 0) return; + /* Surface/client destruction must not close the backing FDs or free the + * imported-buffer record while a GPU read may still be pending. Retain the + * reference without allocating a quarantine node; process exit reclaims it. */ + if (!b || vkp_completion_unknown() || --b->refs > 0) return; vkp_image_destroy(b->img); for (int i = 0; i < b->n_planes; i++) if (b->fd[i] >= 0) close(b->fd[i]); @@ -677,7 +697,8 @@ static void unmap_toplevel(struct surface *s) { /* Let go of the surface's dmabuf content. paced = 1: the buffer was replaced, give it back on the * limiter's cadence; paced = 0: the surface is going away, give it back at once (a buffer of a - * destroyed surface is not shown again and must not sit unreleased with the client). */ + * destroyed surface is not shown again). Completion-unknown overrides both + * release modes: prior GPU reads may outlive the surface. */ static void drop_dmabuf(struct surface *s, int paced) { if (s->dmabuf) { wl_list_remove(&s->dmabuf_destroy.link); @@ -685,7 +706,7 @@ static void drop_dmabuf(struct surface *s, int paced) { * ahb_swapchain.c releases it then. */ if (!ahb_swapchain_defer_release(s->dmabuf_buf, s->dmabuf, s, paced)) { if (paced) release_buffer(s, s->dmabuf, now_ns()); - else wl_buffer_send_release(s->dmabuf); + else send_buffer_release(s->dmabuf); } s->dmabuf = NULL; } @@ -735,7 +756,7 @@ static void take_shm(struct surface *s, struct wl_shm_buffer *shm, struct wl_res vkp_image_upload_shm(s->shm_img, wl_shm_buffer_get_data(shm), stride); wl_shm_buffer_end_access(shm); } - wl_buffer_send_release(buffer); + send_buffer_release(buffer); s->buf_w = w; s->buf_h = h; s->buf_alpha = wl_shm_buffer_get_format(shm) == WL_SHM_FORMAT_ARGB8888; @@ -753,7 +774,7 @@ static void take_pixel(struct surface *s, const uint8_t bgra[4], struct wl_resou } if (!s->shm_img) s->shm_img = vkp_image_create_shm(1, 1); if (s->shm_img) vkp_image_upload_shm(s->shm_img, bgra, 4); - wl_buffer_send_release(buffer); + send_buffer_release(buffer); s->buf_w = 1; s->buf_h = 1; s->buf_alpha = bgra[3] != 0xff; @@ -880,8 +901,9 @@ void **droiddeck_dmabuf_ahb_slot(struct dmabuf_buffer *b) { return &b->ahb_state void droiddeck_dmabuf_ref(struct dmabuf_buffer *b) { b->refs++; } void droiddeck_dmabuf_unref(struct dmabuf_buffer *b) { dmabuf_buffer_unref(b); } void droiddeck_release_buffer(struct surface *s, struct wl_resource *buffer, int paced, int64_t since_ns) { + if (buffer_release_blocked(buffer)) return; if (paced && s) release_buffer(s, buffer, since_ns); - else { wl_buffer_send_release(buffer); perf_note_release(since_ns); } + else { send_buffer_release(buffer); perf_note_release(since_ns); } } void droiddeck_surface_describe(const struct surface *s, char *out, size_t size) { describe(s, out, size); } const struct droiddeck_color *droiddeck_surface_color(const struct surface *s) { @@ -1046,7 +1068,7 @@ static void surface_hold_idle(struct surface *s, struct wl_resource *buffer) { struct wl_shm_buffer *shm = db ? NULL : wl_shm_buffer_get(buffer); int64_t px = db ? (int64_t)db->width * db->height : shm ? (int64_t)wl_shm_buffer_get_width(shm) * wl_shm_buffer_get_height(shm) : 0; - if (px <= 0 || px > CURSOR_MAX_PX) { wl_buffer_send_release(buffer); return; } + if (px <= 0 || px > CURSOR_MAX_PX) { send_buffer_release(buffer); return; } s->idle_buffer = buffer; s->idle_buffer_destroy.notify = on_idle_buffer_destroyed; wl_resource_add_destroy_listener(buffer, &s->idle_buffer_destroy); @@ -1064,7 +1086,7 @@ static struct wl_resource *surface_take_idle(struct surface *s) { static void surface_drop_idle(struct surface *s) { struct wl_resource *buffer = surface_take_idle(s); - if (buffer) wl_buffer_send_release(buffer); + if (buffer) send_buffer_release(buffer); } static void surface_commit(struct wl_client *c, struct wl_resource *r) { @@ -1119,7 +1141,7 @@ static void surface_commit(struct wl_client *c, struct wl_resource *r) { cursor_publish_hidden(); /* wlroots clears its cursor surface to hide the pointer */ } else { cursor_publish_buffer(s, buffer); - wl_buffer_send_release(buffer); + send_buffer_release(buffer); } } else if (s->role == ROLE_NONE) { /* Role-less surface: never drawn, but its buffer is kept for a set_cursor to come. */ @@ -1135,7 +1157,7 @@ static void surface_commit(struct wl_client *c, struct wl_resource *r) { } else { drop_dmabuf(s, 1); s->has_content = 0; - if (buffer) wl_buffer_send_release(buffer); + if (buffer) send_buffer_release(buffer); } } @@ -2322,7 +2344,7 @@ static void pointer_set_cursor(struct wl_client *c, struct wl_resource *r, uint3 struct wl_resource *buffer = surface_take_idle(s); if (buffer) { cursor_publish_buffer(s, buffer); - wl_buffer_send_release(buffer); + send_buffer_release(buffer); } else if (g_cursor_shown == s) { cursor_publish_shown(); } diff --git a/app/src/main/cpp/waylandcomp/src/vk_present.c b/app/src/main/cpp/waylandcomp/src/vk_present.c index dfd3e9f5d..31ed1f6cb 100644 --- a/app/src/main/cpp/waylandcomp/src/vk_present.c +++ b/app/src/main/cpp/waylandcomp/src/vk_present.c @@ -323,6 +323,7 @@ int vkp_has_window(void) { } int vkp_device_lost(void) { return g_dev_state == -2; } +int vkp_completion_unknown(void) { return g_dev_state == -3; } void vk_present_set_scale_mode(int mode, int alignment) { if (mode < VKP_MODE_OFF || mode > VKP_MODE_INTEGER) mode = VKP_MODE_FIT; diff --git a/app/src/main/cpp/waylandcomp/src/vk_present.h b/app/src/main/cpp/waylandcomp/src/vk_present.h index 2a752c343..9506625f1 100644 --- a/app/src/main/cpp/waylandcomp/src/vk_present.h +++ b/app/src/main/cpp/waylandcomp/src/vk_present.h @@ -91,6 +91,10 @@ int vkp_ready(void); int vkp_has_window(void); /* 1 once the Vulkan device was lost: nothing is presented any more (the session must restart). */ int vkp_device_lost(void); +/* Compositor thread only. Submitted reads may still be pending: retain imported + * buffer references and withhold DMA-BUF releases until the app process exits. + * This terminal state is distinct from genuine device loss. */ +int vkp_completion_unknown(void); // Clear to black, blit the draws in order (first = bottom) and present. // Returns 0 on success, -1 if nothing could be presented (no window yet, etc.). diff --git a/docs/development/gpu-pending-completion.md b/docs/development/gpu-pending-completion.md index 3beb4d1dd..db036ba21 100644 --- a/docs/development/gpu-pending-completion.md +++ b/docs/development/gpu-pending-completion.md @@ -28,6 +28,19 @@ without further Vulkan calls. Release fds are still consumed on rejected copies. The normal frame fence still has its existing infinite timeout; this change is not a general GPU hang watchdog. +Externally owned DMA-BUF contents need the same protection. `vkp_completion_unknown()` +exposes the terminal state to compositor-thread lifecycle paths. Replacement, +surface/client destruction, cursor/idle-role releases, previously queued paced +releases and SurfaceFlinger callbacks no longer return a DMA-BUF to its client +after quarantine. The compositor retains DMA-BUF references/backing FDs and AHB +references until app process exit. A display fence alone does not establish that +the compositor's Vulkan reads completed. Queued releases detach destroyed surfaces +and remain held without repeatedly arming an expired timer. New terminal releases +require no allocation, so allocation failure cannot fall back to an unsafe release. +Copied SHM releases and healthy/genuine-device-loss cleanup keep their existing +behaviour. This intentionally prevents further client buffer reuse; restarting +the Android app process, not just the guest session, is required. + ## Regression coverage `python3 -m unittest discover -s tools/tests -p test_vk_pending_completion.py -v` @@ -62,3 +75,14 @@ The change was also exercised in an independently integrated Pixel PowerVR Lab, but its short game observations do not establish the cause of the earlier GNOC reboot or sustained device stability. Main's native CI and APK build remain separate validation. + +`python3 -m unittest discover -s tools/tests -p test_vk_dmabuf_quarantine.py -v` +compiles the production compositor/AHB release, reference and listener functions +verbatim, using the real Wayland list layouts/macros with fake IPC/timer/AHB +endpoints. It covers paced replacement, immediate surface cleanup, already queued +releases, a mixed DMA-BUF/SHM queue, client destruction, a display-fence callback, +healthy cleanup and genuine device loss. It checks withheld releases, retained +FDs/references/AHB ownership, removal of surface listeners and disarmed timers. +The same harness can test an old source directory via +`DROIDDECK_DMABUF_TEST_SOURCE`; it does not inject a real driver fault or prove +long-term device stability. diff --git a/tools/tests/test_vk_dmabuf_quarantine.py b/tools/tests/test_vk_dmabuf_quarantine.py new file mode 100644 index 000000000..b6e035b6a --- /dev/null +++ b/tools/tests/test_vk_dmabuf_quarantine.py @@ -0,0 +1,106 @@ +"""Exercise production DMA-BUF release/ownership paths without a GPU. + +Wayland resources and OS timer/Android AHB endpoints are fake; list macros and +the compositor/AHB lifecycle functions come directly from the repository. +""" +import os +from pathlib import Path +import re +import shutil +import subprocess +import tempfile +import unittest + +from test_vk_pending_completion import function + +ROOT = Path(__file__).resolve().parents[2] +SRC = ROOT / 'app/src/main/cpp/waylandcomp/src' + + +class VkDmabufQuarantineTest(unittest.TestCase): + @classmethod + def setUpClass(cls): + cc = shutil.which(os.environ.get('CC', 'cc')) + if not cc: + raise AssertionError('C compiler required') + # Optional old-head input allows a genuine negative control using the + # identical harness, rather than a model of the previous behaviour. + source_dir = Path(os.environ.get('DROIDDECK_DMABUF_TEST_SOURCE', SRC)) + comp = (source_dir / 'compositor.c').read_text() + ahb = (source_dir / 'ahb_swapchain.c').read_text() + vk = (source_dir / 'vk_present.c').read_text() + names = ['get_dmabuf', 'pending_release_free', + 'pending_releases_forget_surface', 'on_pending_release_buffer_destroyed', + 'arm_release_timer', 'on_release_timer', 'release_buffer', + 'dmabuf_buffer_unref', 'drop_dmabuf', 'on_dmabuf_destroyed', + 'droiddeck_dmabuf_ahb_slot', 'droiddeck_dmabuf_unref', + 'droiddeck_release_buffer'] + helpers = [] + if re.search(r'\bstatic int buffer_release_blocked\(', comp): + helpers = [function(comp, n) for n in ('buffer_release_blocked', 'send_buffer_release')] + else: + helpers = ['static int __attribute__((unused)) buffer_release_blocked(struct wl_resource *b) { (void)b; return 0; }'] + query = function(vk, 'vkp_completion_unknown') if 'int vkp_completion_unknown(' in vk else \ + 'int vkp_completion_unknown(void) { return g_dev_state == -3; }' + comp_functions = [function(comp, names[0])] + helpers + [function(comp, n) for n in names[1:]] + ahb_functions = [function(ahb, n) for n in ('buf_free', 'maybe_free', + 'on_buffer_resource_destroyed', 'send_deferred_release', + 'on_fence_readable', 'ahb_swapchain_defer_release', + 'ahb_swapchain_surface_gone')] + structs = re.search(r'struct pending_release \{.*?\n\};', comp, re.S).group() + '\n' + \ + re.search(r'struct ahb_buf \{.*?\n\};', ahb, re.S).group() + harness = Path(__file__).with_name('vk_dmabuf_quarantine_test.c').read_text() + harness = harness.replace('/* PRODUCTION_STRUCTS */', structs) + harness = harness.replace('/* PRODUCTION_FUNCTIONS */', '\n'.join([query] + comp_functions + ahb_functions)) + cls.tmp = tempfile.TemporaryDirectory(prefix='droiddeck-dmabuf-') + cls.addClassCleanup(cls.tmp.cleanup) + probe = Path(cls.tmp.name) / 'probe.c' + probe.write_text(harness) + cls.binary = probe.with_suffix('') + built = subprocess.run([cc, '-std=c11', '-Wall', '-Wextra', '-Werror', + '-Wno-unused-parameter', '-UNDEBUG', + '-I', str(SRC.parent / 'prebuilt/include'), + str(probe), '-o', str(cls.binary)], capture_output=True, text=True) + if built.returncode: + raise AssertionError(built.stdout + built.stderr) + + def run_scenario(self, name): + result = subprocess.run([str(self.binary), name], capture_output=True, text=True, timeout=10) + self.assertEqual(result.returncode, 0, result.stdout + result.stderr) + + def test_replacement_retains_reference_and_release(self): + self.run_scenario('replacement') + + def test_surface_destroy_retains_reference_and_release(self): + self.run_scenario('surface-gone') + + def test_queued_release_is_held_and_timer_disarmed(self): + self.run_scenario('queued') + + def test_mixed_queue_releases_only_copied_shm(self): + self.run_scenario('mixed') + + def test_client_destroy_retains_backing_fd(self): + self.run_scenario('client-gone') + + def test_ahb_callback_retains_ownership_and_release(self): + self.run_scenario('ahb') + + def test_active_client_destroy_retains_reference(self): + self.run_scenario('active-client-gone') + + def test_healthy_ahb_callback_and_cleanup(self): + self.run_scenario('ahb-healthy') + + def test_device_lost_ahb_callback_and_cleanup(self): + self.run_scenario('ahb-device-lost') + + def test_healthy_release_and_reference_cleanup(self): + self.run_scenario('healthy') + + def test_genuine_device_loss_keeps_existing_cleanup(self): + self.run_scenario('device-lost') + + +if __name__ == '__main__': + unittest.main() diff --git a/tools/tests/test_vk_terminal_callers.py b/tools/tests/test_vk_terminal_callers.py index caacb47a0..7048f5e1b 100644 --- a/tools/tests/test_vk_terminal_callers.py +++ b/tools/tests/test_vk_terminal_callers.py @@ -57,7 +57,7 @@ int __android_log_print(int, const char *, const char *, ...); ''') (tmp / 'wayland-server.h').write_text('#pragma once\nstruct wl_display; struct wl_resource; struct wl_client;\n') compositor = (SOURCE.parent / 'compositor.c').read_text() - callers = '\n'.join(function(compositor, n) for n in ('take_shm', 'take_pixel')) + callers = '\n'.join(function(compositor, n) for n in ('buffer_release_blocked', 'send_buffer_release', 'take_shm', 'take_pixel')) harness = (Path(__file__).with_name('vk_terminal_callers_test.c')).read_text() probe = tmp / 'probe.c' probe.write_text(harness.replace('/* COMPOSITOR_CALLERS */', callers)) diff --git a/tools/tests/vk_dmabuf_quarantine_test.c b/tools/tests/vk_dmabuf_quarantine_test.c new file mode 100644 index 000000000..cbcf724b1 --- /dev/null +++ b/tools/tests/vk_dmabuf_quarantine_test.c @@ -0,0 +1,199 @@ +/* Production lifecycle functions are inserted verbatim by Python. No GPU, + * Android app, external client or real timer is involved in this probe. */ +#define _POSIX_C_SOURCE 200809L +#include +#include +#include +#include +#include +#include +#include +#include +#include +#ifndef __linux__ +struct itimerspec { struct timespec it_interval, it_value; }; +#endif + +struct wl_listener { struct wl_list link; void (*notify)(struct wl_listener *, void *); }; +struct wl_resource { int dma; void *user_data; struct wl_list listeners; unsigned releases; }; +struct wl_client; +struct wl_event_source { int unused; }; +typedef struct AHardwareBuffer { int unused; } AHardwareBuffer; +struct vkp_image { int unused; }; +struct dmabuf_buffer { + int refs, n_planes, fd[4]; + struct vkp_image *img; + void *ahb_state; +}; +struct surface { + struct wl_resource *dmabuf; + struct dmabuf_buffer *dmabuf_buf; + struct wl_listener dmabuf_destroy; + int releases_pending; + int64_t next_release_ns; +}; +/* PRODUCTION_STRUCTS */ +static int g_dev_state, g_fps_limit, g_release_timer_fd; +static int64_t g_refresh_ns; +static struct wl_list g_pending_releases, g_bufs; +static void *g_display; +static unsigned image_destroys, ahb_releases, source_removes; +static struct itimerspec last_timer; +static const int wl_buffer_interface = 1, dbuf_buffer_impl = 2; +#define TFD_TIMER_ABSTIME 1 + +/* List operations use the real Wayland layouts/macros. OS/IPC endpoints alone + * are fakes, so no event is sent and no process or hardware is touched. */ +void wl_list_init(struct wl_list *l) { l->prev = l; l->next = l; } +void wl_list_insert(struct wl_list *l, struct wl_list *e) { + e->next = l->next; e->prev = l; l->next->prev = e; l->next = e; +} +void wl_list_remove(struct wl_list *e) { e->next->prev = e->prev; e->prev->next = e->next; } +static int wl_resource_instance_of(struct wl_resource *r, const void *i, const void *impl) { return r->dma; } +static void *wl_resource_get_user_data(struct wl_resource *r) { return r->user_data; } +static void wl_resource_add_destroy_listener(struct wl_resource *r, struct wl_listener *l) { + wl_list_insert(r->listeners.prev, &l->link); +} +static void wl_buffer_send_release(struct wl_resource *r) { ++r->releases; } +static void wl_display_flush_clients(void *d) {} +static int timerfd_settime(int fd, int flags, const struct itimerspec *v, void *old) { last_timer = *v; return 0; } +static void perf_note_release(int64_t since) {} +static int64_t now_ns(void) { return 1000000000LL; } +static void vkp_image_destroy(struct vkp_image *img) { ++image_destroys; } +static void AHardwareBuffer_release(AHardwareBuffer *ahb) { ++ahb_releases; } +static int wl_event_source_remove(struct wl_event_source *src) { ++source_removes; return 0; } +static struct dmabuf_buffer *get_dmabuf(struct wl_resource *r); +int ahb_swapchain_defer_release(struct dmabuf_buffer *, struct wl_resource *, struct surface *, int); +/* PRODUCTION_FUNCTIONS */ + +static void init_resource(struct wl_resource *r, struct dmabuf_buffer *b) { + *r = (struct wl_resource){.dma = b != NULL, .user_data = b}; + wl_list_init(&r->listeners); +} +static struct dmabuf_buffer *init_buffer(struct surface *s, struct wl_resource *r) { + struct dmabuf_buffer *b = calloc(1, sizeof(*b)); assert(b); + b->refs = 2; b->n_planes = 1; + int fds[2]; assert(pipe(fds) == 0); b->fd[0] = fds[0]; close(fds[1]); + init_resource(r, b); + assert(get_dmabuf(r) == b); + *s = (struct surface){.dmabuf = r, .dmabuf_buf = b}; + s->dmabuf_destroy.notify = on_dmabuf_destroyed; + wl_resource_add_destroy_listener(r, &s->dmabuf_destroy); + return b; +} +static void destroy_resource(struct wl_resource *r) { + while (r->listeners.next != &r->listeners) { + struct wl_listener *l = wl_container_of(r->listeners.next, l, link); + l->notify(l, r); + } + if (r->dma) dmabuf_buffer_unref(r->user_data); + r->user_data = NULL; +} +static void fire_timer(void) { + int fd[2]; uint64_t expirations = 1; + assert(pipe(fd) == 0 && write(fd[1], &expirations, sizeof(expirations)) == sizeof(expirations)); + on_release_timer(fd[0], 0, NULL); close(fd[0]); close(fd[1]); +} +static void assert_retained(struct dmabuf_buffer *b, int refs) { + assert(b->refs == refs && fcntl(b->fd[0], F_GETFD) >= 0 && !image_destroys); +} +static void test_drop(int paced) { + struct surface s; struct wl_resource r; + struct dmabuf_buffer *b = init_buffer(&s, &r); + g_dev_state = -3; drop_dmabuf(&s, paced); + assert(!s.dmabuf && !s.dmabuf_buf && !r.releases && !s.releases_pending); + assert(r.listeners.next == &r.listeners); /* no listener into a dead surface */ + assert_retained(b, 2); + destroy_resource(&r); assert_retained(b, 2); + close(b->fd[0]); free(b); /* fake GPU only */ +} +static void test_queue(int mixed, int client_gone) { + struct surface s; struct wl_resource r, shm; + struct dmabuf_buffer *b = init_buffer(&s, &r); + g_release_timer_fd = 123; g_refresh_ns = 100000; + drop_dmabuf(&s, 1); assert(s.releases_pending == 1 && !r.releases); + assert(last_timer.it_value.tv_sec || last_timer.it_value.tv_nsec); + if (mixed) { init_resource(&shm, NULL); release_buffer(&s, &shm, now_ns()); } + pending_releases_forget_surface(&s); + g_dev_state = -3; + fire_timer(); + assert(!r.releases && g_pending_releases.next != &g_pending_releases); + if (mixed) assert(shm.releases == 1); + assert(!last_timer.it_value.tv_sec && !last_timer.it_value.tv_nsec); + assert_retained(b, 1); + if (client_gone) { + destroy_resource(&r); + assert(g_pending_releases.next == &g_pending_releases); + assert_retained(b, 1); + } + close(b->fd[0]); free(b); /* fake GPU only */ +} +static void test_active_client_destroy(void) { + struct surface s; struct wl_resource r; + struct dmabuf_buffer *b = init_buffer(&s, &r); + g_dev_state = -3; + destroy_resource(&r); + assert(!s.dmabuf && s.dmabuf_buf == b && !r.releases); + drop_dmabuf(&s, 0); assert(!s.dmabuf_buf); + assert_retained(b, 2); + close(b->fd[0]); free(b); /* fake GPU only */ +} +static void test_ahb(int terminal, int lost) { + struct surface s; struct wl_resource r; + struct dmabuf_buffer *b = init_buffer(&s, &r); + struct ahb_buf *ab = calloc(1, sizeof(*ab)); assert(ab); + AHardwareBuffer ahb; + *ab = (struct ahb_buf){.b = b, .ahb = &ahb, .resource = &r, .on_layer = 1, .fence_fd = -1}; + b->ahb_state = ab; ++b->refs; + ab->resource_destroy.notify = on_buffer_resource_destroyed; + wl_resource_add_destroy_listener(&r, &ab->resource_destroy); + wl_list_insert(&g_bufs, &ab->link); + drop_dmabuf(&s, 1); assert(ab->release_pending && ab->surface == &s); + int backing_fd = b->fd[0]; + g_dev_state = terminal ? -3 : lost ? -2 : 1; + ahb_swapchain_surface_gone(&s); assert(!ab->surface); + /* Actual event-loop callback after the display's fence becomes readable. */ + struct wl_event_source source; int fds[2]; assert(pipe(fds) == 0); + ab->on_layer = 0; ab->fence_src = &source; ab->fence_fd = fds[0]; + on_fence_readable(fds[0], 0, ab); close(fds[1]); + assert(source_removes == 1 && ab->fence_fd == -1 && !ab->fence_src); + assert(r.releases == (terminal ? 0u : 1u)); + assert(ab->release_pending == terminal && !ahb_releases); + destroy_resource(&r); + if (terminal) { + assert(!ab->resource && b->ahb_state == ab && !ahb_releases); + assert_retained(b, 2); + close(b->fd[0]); free(ab); free(b); /* fake GPU only */ + } else { + assert(ahb_releases == 1 && image_destroys == 1); + errno = 0; assert(fcntl(backing_fd, F_GETFD) == -1 && errno == EBADF); + } +} +static void test_cleanup(int lost) { + struct surface s; struct wl_resource r; + struct dmabuf_buffer *b = init_buffer(&s, &r); + int fd = b->fd[0]; g_dev_state = lost ? -2 : 1; + assert(!vkp_completion_unknown()); + drop_dmabuf(&s, 0); + assert(r.releases == 1 && b->refs == 1 && !image_destroys); + destroy_resource(&r); assert(image_destroys == 1); + errno = 0; assert(fcntl(fd, F_GETFD) == -1 && errno == EBADF); +} +int main(int argc, char **argv) { + assert(argc == 2); + wl_list_init(&g_pending_releases); wl_list_init(&g_bufs); + g_dev_state = 1; g_release_timer_fd = -1; + if (!strcmp(argv[1], "replacement")) test_drop(1); + else if (!strcmp(argv[1], "surface-gone")) test_drop(0); + else if (!strcmp(argv[1], "queued")) test_queue(0, 0); + else if (!strcmp(argv[1], "mixed")) test_queue(1, 0); + else if (!strcmp(argv[1], "client-gone")) test_queue(0, 1); + else if (!strcmp(argv[1], "ahb")) test_ahb(1, 0); + else if (!strcmp(argv[1], "ahb-healthy")) test_ahb(0, 0); + else if (!strcmp(argv[1], "ahb-device-lost")) test_ahb(0, 1); + else if (!strcmp(argv[1], "active-client-gone")) test_active_client_destroy(); + else if (!strcmp(argv[1], "healthy")) test_cleanup(0); + else if (!strcmp(argv[1], "device-lost")) test_cleanup(1); + else assert(0 && "unknown scenario"); + return 0; +} diff --git a/tools/tests/vk_terminal_callers_test.c b/tools/tests/vk_terminal_callers_test.c index e395df07d..d9364e318 100644 --- a/tools/tests/vk_terminal_callers_test.c +++ b/tools/tests/vk_terminal_callers_test.c @@ -108,6 +108,10 @@ static void wl_shm_buffer_end_access(struct wl_shm_buffer *b) {} static void wl_buffer_send_release(struct wl_resource *b) { ++releases; } static struct wl_client *wl_resource_get_client(struct wl_resource *r) { return NULL; } static struct client_info *client_info_of(struct wl_client *c) { return NULL; } +/* This harness exercises copied SHM/pixel callers; DMA-BUF lifecycle paths are + * compiled separately by test_vk_dmabuf_quarantine.py. */ +struct dmabuf_buffer; +static struct dmabuf_buffer *get_dmabuf(struct wl_resource *buffer) { return NULL; } /* COMPOSITOR_CALLERS */ static void setup(int extra) { @@ -174,7 +178,7 @@ static void acquire_failure(int later, VkResult result) { idle_answer = VK_ERROR_OUT_OF_HOST_MEMORY; VkImage *retained = g_images; assert(vkp_render(64, 64, NULL, 0) == -1); - assert(g_dev_state == -3 && !vkp_device_lost()); + assert(g_dev_state == -3 && vkp_completion_unknown() && !vkp_device_lost()); assert(acquires == fail_acquire_at && idle_calls == 1); assert(submits == (unsigned)later && markers == 0 && waits == 0); assert(destroys == 0 && g_images == retained && g_nimg == 1); @@ -234,6 +238,7 @@ int main(int argc, char **argv) { if (!strcmp(name, "acquire-device-lost")) { fail_acquire_at = 2; acquire_answer = VK_ERROR_DEVICE_LOST; } else present_answer = VK_ERROR_DEVICE_LOST; assert(vkp_render(64, 64, NULL, 0) == -1 && vkp_device_lost()); + assert(!vkp_completion_unknown()); assert(g_dev_state == -2 && lost_notifications == 1 && markers == 0 && waits == 0); assert(!g_images && !g_swapchain && !g_surface && destroys == 2); assert_stopped();