Retain compositor resources when GPU completion is unconfirmed

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;

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.

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 <assert.h>
+#include <stdint.h>
+#include <stdlib.h>
+#include <stdarg.h>
+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 <assert.h>
+#include <stdint.h>
+#include <stdlib.h>
+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()