From 720625c99a729a5c24a9e391e322ad6241517789 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 01:10:54 -0300 Subject: [PATCH 1/8] feat: resolve the renderer at launch and export LABELLE_BGFX_RENDERER (#27) launch_intent.apply now ends with renderer.resolve(activity, extra), which decides the Android renderer before bgfx starts (labelle-bgfx#172 D1-D3): debuggable-only LABELLE_BGFX_RENDERER intent extra -> crash guard (D11 stub, labelle-android#28) -> labelle.renderer manifest meta-data (#26; missing or invalid -> gles) -> auto (Vulkan 1.1 system feature, else gles). It sets LABELLE_BGFX_RENDERER and logs "renderer: (source: ...)". - src/renderer.zig: pure decide() with host tests (fakes), Android glue - src/crash_guard.zig: stub seam, always "not disabled" - src/jni/renderer_query.c: meta-data read + hasSystemFeature via JNI - src/intent_env.zig: LABELLE_BGFX_RENDERER on the allow-list (debuggable only) Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- .github/workflows/ci.yml | 2 +- build.zig | 1 + src/crash_guard.zig | 25 +++ src/intent_env.zig | 40 ++++- src/jni/renderer_query.c | 158 +++++++++++++++++++ src/launch_intent.zig | 10 ++ src/renderer.zig | 333 +++++++++++++++++++++++++++++++++++++++ src/root.zig | 7 + 8 files changed, 572 insertions(+), 4 deletions(-) create mode 100644 src/crash_guard.zig create mode 100644 src/jni/renderer_query.c create mode 100644 src/renderer.zig diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d3ea12f..5a5aaf7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,7 +66,7 @@ jobs: uses: nttld/setup-ndk@v1 with: ndk-version: r27c - # Compiles the three JNI C TUs against the NDK sysroot and the module's + # Compiles the JNI C TUs against the NDK sysroot and the module's # `extern "c"` / AAudio / MediaCodec bindings for aarch64 (object # emission; nothing runs). - name: Cross-build for Android diff --git a/build.zig b/build.zig index aaf6106..32879f2 100644 --- a/build.zig +++ b/build.zig @@ -136,6 +136,7 @@ pub fn build(b: *std.Build) void { "src/jni/intent_extras.c", "src/jni/debuggable.c", "src/jni/window_relayout.c", + "src/jni/renderer_query.c", }, .flags = &.{ "-std=c11", "-Wall" }, }); diff --git a/src/crash_guard.zig b/src/crash_guard.zig new file mode 100644 index 0000000..1e85509 --- /dev/null +++ b/src/crash_guard.zig @@ -0,0 +1,25 @@ +//! Vulkan crash guard (labelle-bgfx#172 D11): the SEAM only. +//! +//! D11: mark "Vulkan start in progress" in the app's internal storage before a +//! Vulkan init, clear it once the game has run stably, and start on GLES at +//! the next launch if the mark is still there. That work is +//! labelle-toolkit/labelle-android#28. Until it lands this is a stub that +//! never disables Vulkan, so `renderer.resolve` (#27) already consults the +//! guard in the right place (after the intent override, before the provider +//! setting) and #28 only has to replace the body below. +const std = @import("std"); + +/// True when a previous launch crashed during a Vulkan start and this launch +/// must use GLES instead. `activity` is the running `ANativeActivity*` +/// (opaque), for the internal-storage path #28 will need. +/// +/// Stub: always "not disabled". +pub fn isVulkanDisabled(activity: ?*const anyopaque) bool { + _ = activity; + return false; +} + +test "stub: Vulkan is never disabled" { + try std.testing.expect(!isVulkanDisabled(null)); + try std.testing.expect(!isVulkanDisabled(@ptrFromInt(0x1000))); +} diff --git a/src/intent_env.zig b/src/intent_env.zig index 9ba77b5..27bbb11 100644 --- a/src/intent_env.zig +++ b/src/intent_env.zig @@ -39,8 +39,21 @@ pub const keys = [_]Key{ .{ .name = "LABELLE_PROFILE", .debuggable_only = false }, .{ .name = "LABELLE_SCREENSHOT_PATH", .debuggable_only = true }, .{ .name = "LABELLE_SCREENSHOT_AFTER_SEC", .debuggable_only = true }, + // The renderer override (labelle-android#27, labelle-bgfx#172 D3): a + // test/debug channel, so debuggable builds only. `renderer.zig` decides + // the final `LABELLE_BGFX_RENDERER` after this runs (and re-applies the + // same gate and validates the value). + .{ .name = "LABELLE_BGFX_RENDERER", .debuggable_only = true }, }; +/// Index of `name` in `keys` (comptime: a typo is a compile error). +pub fn indexOf(comptime name: []const u8) usize { + inline for (keys, 0..) |k, i| { + if (comptime std.mem.eql(u8, k.name, name)) return i; + } + @compileError("not an allow-listed intent key: " ++ name); +} + /// Is `name` one of the keys copied from the intent? pub fn isAllowed(name: []const u8) bool { for (keys) |k| { @@ -189,15 +202,17 @@ fn extrasWith(pairs: []const struct { [:0]const u8, [:0]const u8 }) [keys.len]?[ return out; } -test "the allow-list is exactly the four run-option keys" { +test "the allow-list is exactly the four run-option keys plus the renderer override" { try testing.expect(isAllowed("LABELLE_SCENE")); try testing.expect(isAllowed("LABELLE_PROFILE")); try testing.expect(isAllowed("LABELLE_SCREENSHOT_PATH")); try testing.expect(isAllowed("LABELLE_SCREENSHOT_AFTER_SEC")); - try testing.expectEqual(@as(usize, 4), keys.len); + try testing.expect(isAllowed("LABELLE_BGFX_RENDERER")); + try testing.expect(keys[indexOf("LABELLE_BGFX_RENDERER")].debuggable_only); + try testing.expectEqual(@as(usize, 5), keys.len); // Other labelle knobs and arbitrary env names are not copied. try testing.expect(!isAllowed("LABELLE_FIXED_DT")); - try testing.expect(!isAllowed("LABELLE_BGFX_RENDERER")); + try testing.expect(!isAllowed("LABELLE_BGFX_BACKEND")); try testing.expect(!isAllowed("LD_PRELOAD")); try testing.expect(!isAllowed("PATH")); // Exact match only: no prefix, case or whitespace slack. @@ -248,6 +263,25 @@ test "screenshot extras need a debuggable apk; debuggable is asked once" { try testing.expectEqual(@as(usize, 1), debug.debuggable_calls); } +test "the renderer override needs a debuggable apk and is reverted on a plain relaunch" { + const extras = extrasWith(&.{.{ "LABELLE_BGFX_RENDERER", "vulkan" }}); + + var state: State = .{}; + var release: FakeEnv = .{ .is_debuggable = false }; + apply(&state, extras, &release); + try testing.expect(release.get("LABELLE_BGFX_RENDERER") == null); + try testing.expectEqual(@as(usize, 1), release.debuggable_calls); + + var state2: State = .{}; + var debug: FakeEnv = .{ .is_debuggable = true }; + apply(&state2, extras, &debug); + try testing.expectEqualStrings("vulkan", debug.get("LABELLE_BGFX_RENDERER").?); + try testing.expect(state2.set_by_intent[indexOf("LABELLE_BGFX_RENDERER")]); + apply(&state2, @splat(null), &debug); + try testing.expect(debug.get("LABELLE_BGFX_RENDERER") == null); + try testing.expect(!state2.set_by_intent[indexOf("LABELLE_BGFX_RENDERER")]); +} + test "an empty extra is treated as absent" { var state: State = .{}; var env: FakeEnv = .{}; diff --git a/src/jni/renderer_query.c b/src/jni/renderer_query.c new file mode 100644 index 0000000..6fd9174 --- /dev/null +++ b/src/jni/renderer_query.c @@ -0,0 +1,158 @@ +// The two device/package questions the launch-time renderer resolution asks +// (labelle-android#27, labelle-bgfx#172 D1-D3). The pure decision is +// `renderer.zig`; this is the JNI half: +// +// * the provider setting the packager stamps into the manifest +// (``, +// labelle-android#26): +// getPackageManager() +// .getApplicationInfo(getPackageName(), GET_META_DATA) +// .metaData.getString("labelle.renderer") +// * `auto`'s device check: +// getPackageManager().hasSystemFeature("android.hardware.vulkan.version", +// 0x401000) // Vulkan 1.1 +// +// In C for the same reason as intent_extras.c: already declares the +// JNI vtables. Off Android this is an empty TU. +#ifdef __ANDROID__ + +#include +#include +#include +#include + +// android.content.pm.PackageManager.GET_META_DATA (API 1). +#define LABELLE_GET_META_DATA 0x00000080 + +// intent_extras.c: attach-if-detached, reporting what we attached. +JNIEnv *labelle_android_acquire_env(JavaVM *vm, int *we_attached); + +// `activity.getPackageManager()`, or NULL (possibly with an exception +// pending; the callers clear it). A local ref inside the caller's frame. +static jobject package_manager(JNIEnv *env, jobject activity) { + jclass activity_cls = (*env)->GetObjectClass(env, activity); + if (activity_cls == NULL) return NULL; + jmethodID get_pm = (*env)->GetMethodID(env, activity_cls, "getPackageManager", "()Landroid/content/pm/PackageManager;"); + if (get_pm == NULL || (*env)->ExceptionCheck(env)) return NULL; + jobject pm = (*env)->CallObjectMethod(env, activity, get_pm); + if ((*env)->ExceptionCheck(env)) return NULL; + return pm; +} + +// Copy the `labelle.renderer` meta-data string into `buf` (NUL-terminated). +// Returns its length (>= 0), -1 when the key is absent (or not a string), +// -2 on any JNI failure, -3 when the value does not fit in `buf_cap`. +// Never leaves a Java exception pending and never leaks a local ref. +// Callable from any thread (attached or not). +int labelle_android_read_renderer_meta(const void *activity_ptr, char *buf, size_t buf_cap) { + const ANativeActivity *na = (const ANativeActivity *)activity_ptr; + if (na == NULL || na->vm == NULL || na->clazz == NULL || buf == NULL || buf_cap == 0) return -2; + JavaVM *vm = na->vm; + jobject activity = na->clazz; + + int we_attached = 0; + JNIEnv *env = labelle_android_acquire_env(vm, &we_attached); + if (env == NULL) return -2; + + int result = -2; + if ((*env)->PushLocalFrame(env, 16) == JNI_OK) { + jobject pm = package_manager(env, activity); + jclass activity_cls = (pm != NULL) ? (*env)->GetObjectClass(env, activity) : NULL; + jmethodID get_pkg = activity_cls ? (*env)->GetMethodID(env, activity_cls, "getPackageName", "()Ljava/lang/String;") : NULL; + jstring pkg = (get_pkg && !(*env)->ExceptionCheck(env)) ? (jstring)(*env)->CallObjectMethod(env, activity, get_pkg) : NULL; + jclass pm_cls = (pkg && !(*env)->ExceptionCheck(env)) ? (*env)->GetObjectClass(env, pm) : NULL; + // The (String, int) overload: deprecated from API 33 in favour of + // the ApplicationInfoFlags one, but still present and working. + jmethodID get_app_info = pm_cls ? (*env)->GetMethodID(env, pm_cls, "getApplicationInfo", "(Ljava/lang/String;I)Landroid/content/pm/ApplicationInfo;") : NULL; + // Throws NameNotFoundException only if our own package vanished. + jobject app_info = (get_app_info && !(*env)->ExceptionCheck(env)) ? (*env)->CallObjectMethod(env, pm, get_app_info, pkg, (jint)LABELLE_GET_META_DATA) : NULL; + if (app_info != NULL && !(*env)->ExceptionCheck(env)) { + jclass app_info_cls = (*env)->GetObjectClass(env, app_info); + jfieldID meta_fid = app_info_cls ? (*env)->GetFieldID(env, app_info_cls, "metaData", "Landroid/os/Bundle;") : NULL; + if (meta_fid != NULL && !(*env)->ExceptionCheck(env)) { + jobject bundle = (*env)->GetObjectField(env, app_info, meta_fid); + if (bundle == NULL) { + // No at all in . + result = -1; + } else { + jclass bundle_cls = (*env)->GetObjectClass(env, bundle); + jmethodID get_string = bundle_cls ? (*env)->GetMethodID(env, bundle_cls, "getString", "(Ljava/lang/String;)Ljava/lang/String;") : NULL; + jstring jkey = (get_string && !(*env)->ExceptionCheck(env)) ? (*env)->NewStringUTF(env, "labelle.renderer") : NULL; + jstring jval = (jkey && !(*env)->ExceptionCheck(env)) ? (jstring)(*env)->CallObjectMethod(env, bundle, get_string, jkey) : NULL; + if (jkey != NULL && !(*env)->ExceptionCheck(env)) { + if (jval == NULL) { + // Missing key, or a non-string value. + result = -1; + } else { + // The accepted values are ASCII, so modified + // UTF-8 is byte-identical for every valid one. + jsize chars = (*env)->GetStringLength(env, jval); + jsize bytes = (*env)->GetStringUTFLength(env, jval); + if (bytes < 0 || (size_t)bytes + 1 > buf_cap) { + result = -3; + } else { + (*env)->GetStringUTFRegion(env, jval, 0, chars, buf); + if (!(*env)->ExceptionCheck(env)) { + buf[bytes] = 0; + result = (int)bytes; + } + } + } + } + } + } + } + // Any JNI call above may have raised; clear before popping so we + // never hand a pending exception back to the caller's thread. + if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + result = -2; + } + (*env)->PopLocalFrame(env, NULL); + } else if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + __android_log_print(ANDROID_LOG_WARN, "labelle-android", "renderer meta-data: PushLocalFrame failed; exception cleared"); + } + + if (we_attached) (*vm)->DetachCurrentThread(vm); + return result; +} + +// `PackageManager.hasSystemFeature(name, version)` (API 24+). Returns 1 when +// the device reports the feature at `version` or above, 0 otherwise — and 0 +// on any JNI failure: `auto` then falls back to GLES, the safe choice. +int labelle_android_has_system_feature(const void *activity_ptr, const char *name, int version) { + const ANativeActivity *na = (const ANativeActivity *)activity_ptr; + if (na == NULL || na->vm == NULL || na->clazz == NULL || name == NULL) return 0; + JavaVM *vm = na->vm; + jobject activity = na->clazz; + + int we_attached = 0; + JNIEnv *env = labelle_android_acquire_env(vm, &we_attached); + if (env == NULL) return 0; + + int has = 0; + if ((*env)->PushLocalFrame(env, 8) == JNI_OK) { + jobject pm = package_manager(env, activity); + jclass pm_cls = (pm && !(*env)->ExceptionCheck(env)) ? (*env)->GetObjectClass(env, pm) : NULL; + jmethodID has_feature = pm_cls ? (*env)->GetMethodID(env, pm_cls, "hasSystemFeature", "(Ljava/lang/String;I)Z") : NULL; + jstring jname = (has_feature && !(*env)->ExceptionCheck(env)) ? (*env)->NewStringUTF(env, name) : NULL; + if (jname != NULL && !(*env)->ExceptionCheck(env)) { + jboolean r = (*env)->CallBooleanMethod(env, pm, has_feature, jname, (jint)version); + if (!(*env)->ExceptionCheck(env)) has = r ? 1 : 0; + } + if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + has = 0; + } + (*env)->PopLocalFrame(env, NULL); + } else if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + __android_log_print(ANDROID_LOG_WARN, "labelle-android", "system feature: PushLocalFrame failed; exception cleared"); + } + + if (we_attached) (*vm)->DetachCurrentThread(vm); + return has; +} + +#endif /* __ANDROID__ */ diff --git a/src/launch_intent.zig b/src/launch_intent.zig index 4670089..c71276e 100644 --- a/src/launch_intent.zig +++ b/src/launch_intent.zig @@ -26,10 +26,16 @@ //! which only that loop delivers, so the env is complete first. //! * sokol: from the top of the generated `sokol_main()` on the UI thread //! (already attached), before the render thread that runs `init` exists. +//! +//! `apply` ends with `renderer.resolve` (labelle-android#27), which decides +//! and exports `LABELLE_BGFX_RENDERER`; it runs last so its value is final +//! (it overrides whatever the raw intent extra set) and, like the extras, it +//! lands before bgfx is initialised on the first INIT_WINDOW. const std = @import("std"); const intent_env = @import("intent_env.zig"); const debuggable_mod = @import("debuggable.zig"); +const renderer = @import("renderer.zig"); const is_android = @import("root.zig").is_android; // `jni/intent_extras.c` — only referenced on Android, where build.zig @@ -90,6 +96,7 @@ pub fn apply(activity: ?*const anyopaque) void { // Still revert what an earlier launch's intent set in this process // (all-absent extras only ever revert/keep, never set). intent_env.apply(&state, @splat(null), LibcEnv{ .activity = a }); + renderer.resolve(a, null); return; } var extras: [intent_env.keys.len]?[:0]const u8 = @splat(null); @@ -102,6 +109,9 @@ pub fn apply(activity: ?*const anyopaque) void { off += n + 1; } intent_env.apply(&state, extras, LibcEnv{ .activity = a }); + // LAST: the renderer decision (intent override → crash guard → provider + // setting → auto) overwrites the raw extra with the validated value. + renderer.resolve(a, extras[intent_env.indexOf(renderer.env_name)]); } test "apply is a no-op off Android (and never touches the externs)" { diff --git a/src/renderer.zig b/src/renderer.zig new file mode 100644 index 0000000..8916edd --- /dev/null +++ b/src/renderer.zig @@ -0,0 +1,333 @@ +//! Launch-time renderer resolution (labelle-android#27; labelle-bgfx#172 +//! D1-D3, D11). +//! +//! labelle-android owns the Android renderer POLICY; labelle-bgfx only reads +//! `LABELLE_BGFX_RENDERER` before `bgfx.init` (labelle-bgfx#176). This file +//! decides the value and exports it, once per activity launch. +//! +//! Resolution order, first match wins: +//! 1. `intent`: the `LABELLE_BGFX_RENDERER` launch extra (`vulkan`|`gles`), +//! only in a debuggable apk (the same gate as the other debuggable-only +//! keys in `intent_env.zig`). Anything else is warned about and ignored. +//! 2. `crash-guard`: Vulkan disabled after a crashed Vulkan start (D11, +//! `crash_guard.zig`; a stub until labelle-android#28) → `gles`. +//! 3. `setting`: the provider setting, stamped into the manifest as +//! `` +//! (labelle-android#26). Missing or unreadable → `gles`; an invalid +//! value → `gles` with a warning. +//! 4. `auto` (the setting's third value): Vulkan when the device reports +//! `android.hardware.vulkan.version` >= 1.1 (0x401000), else `gles`. +//! +//! Then `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles", 1)` and one log +//! line: `renderer: (source: intent|crash-guard|setting|auto)`. +//! +//! ## Where it runs +//! +//! `launch_intent.apply` calls `resolve` LAST, so it runs from the same entry +//! point the backends already call before anything reads the environment: +//! bgfx calls `apply` at the top of `run(app)`, before the event loop that +//! delivers the first INIT_WINDOW, and bgfx is only initialised inside that +//! loop, so the variable is always set before `bgfx.init`. +//! +//! `decide` is the PURE half (host-tested with fakes below); `resolve` is the +//! Android glue (JNI in `jni/renderer_query.c`, libc `setenv`). +const std = @import("std"); +const debuggable_mod = @import("debuggable.zig"); +const crash_guard = @import("crash_guard.zig"); +const is_android = @import("root.zig").is_android; + +/// The environment variable labelle-bgfx reads before `bgfx.init`. +pub const env_name: [:0]const u8 = "LABELLE_BGFX_RENDERER"; +/// The manifest `` name the packager stamps (labelle-android#26). +pub const meta_data_name = "labelle.renderer"; +/// `android.hardware.vulkan.version` for Vulkan 1.1 (D2, D7). +pub const vulkan_1_1: c_int = 0x401000; + +/// What is exported in `LABELLE_BGFX_RENDERER`. +pub const Renderer = enum { + gles, + vulkan, + + pub fn envValue(self: Renderer) [:0]const u8 { + return switch (self) { + .gles => "gles", + .vulkan => "vulkan", + }; + } +}; + +/// Which rule decided (the `source:` in the log line). +pub const Source = enum { + intent, + crash_guard, + setting, + auto, + + pub fn label(self: Source) []const u8 { + return switch (self) { + .intent => "intent", + .crash_guard => "crash-guard", + .setting => "setting", + .auto => "auto", + }; + } +}; + +pub const Decision = struct { renderer: Renderer, source: Source }; + +/// The provider setting's values. +pub const Setting = enum { gles, vulkan, auto }; + +/// An explicit renderer (the intent extra). Exact, lower-case match only. +pub fn parseRenderer(value: []const u8) ?Renderer { + if (std.mem.eql(u8, value, "vulkan")) return .vulkan; + if (std.mem.eql(u8, value, "gles")) return .gles; + return null; +} + +/// The `labelle.renderer` meta-data. Exact, lower-case match only (the +/// provider setting is parsed strictly, so a valid APK only carries these). +pub fn parseSetting(value: []const u8) ?Setting { + if (std.mem.eql(u8, value, "auto")) return .auto; + if (parseRenderer(value)) |r| return switch (r) { + .gles => .gles, + .vulkan => .vulkan, + }; + return null; +} + +/// Decide the renderer. `q` answers, lazily and in this order, only what the +/// rules need: +/// * `intentExtra() ?[]const u8` — the launch extra (null = absent) +/// * `debuggable() bool` — asked only when the extra is present +/// * `vulkanDisabled() bool` — the D11 crash guard +/// * `metaData() ?[]const u8` — the `labelle.renderer` meta-data (null = +/// missing or unreadable) +/// * `hasVulkan() bool` — asked only for `auto` +pub fn decide(q: anytype) Decision { + if (q.intentExtra()) |v| { + // An empty extra counts as absent, as it does in `intent_env`. + if (v.len > 0) { + if (!q.debuggable()) { + std.log.info("android: ignoring intent extra {s}: the apk is not debuggable", .{env_name}); + } else if (parseRenderer(v)) |r| { + return .{ .renderer = r, .source = .intent }; + } else { + std.log.warn("android: ignoring intent extra {s}='{s}' (expected 'vulkan' or 'gles')", .{ env_name, v }); + } + } + } + if (q.vulkanDisabled()) return .{ .renderer = .gles, .source = .crash_guard }; + const setting: Setting = if (q.metaData()) |raw| + parseSetting(raw) orelse blk: { + std.log.warn("android: invalid {s} meta-data '{s}' (expected 'gles', 'vulkan' or 'auto'); using gles", .{ meta_data_name, raw }); + break :blk .gles; + } + else + .gles; + return switch (setting) { + .gles => .{ .renderer = .gles, .source = .setting }, + .vulkan => .{ .renderer = .vulkan, .source = .setting }, + .auto => .{ .renderer = if (q.hasVulkan()) .vulkan else .gles, .source = .auto }, + }; +} + +// ── Android glue ──────────────────────────────────────────────────────── + +// `jni/renderer_query.c` — only referenced on Android, where build.zig +// compiles the C TU into this module. +extern "c" fn labelle_android_read_renderer_meta(activity: ?*const anyopaque, buf: [*]u8, buf_cap: usize) c_int; +extern "c" fn labelle_android_has_system_feature(activity: ?*const anyopaque, name: [*:0]const u8, version: c_int) c_int; +extern "c" fn setenv(name: [*:0]const u8, value: [*:0]const u8, overwrite: c_int) c_int; + +const JniQuery = struct { + activity: *const anyopaque, + extra: ?[:0]const u8, + meta_buf: [64]u8 = undefined, + + pub fn intentExtra(self: *JniQuery) ?[]const u8 { + return self.extra; + } + pub fn debuggable(self: *JniQuery) bool { + return debuggable_mod.isDebuggable(self.activity); + } + pub fn vulkanDisabled(self: *JniQuery) bool { + return crash_guard.isVulkanDisabled(self.activity); + } + pub fn metaData(self: *JniQuery) ?[]const u8 { + const n = labelle_android_read_renderer_meta(self.activity, &self.meta_buf, self.meta_buf.len); + switch (n) { + -1 => { + std.log.info("android: no {s} meta-data; using gles", .{meta_data_name}); + return null; + }, + -3 => { + std.log.warn("android: {s} meta-data too long; using gles", .{meta_data_name}); + return null; + }, + else => if (n < 0) { + std.log.warn("android: could not read the {s} meta-data; using gles", .{meta_data_name}); + return null; + }, + } + return self.meta_buf[0..@intCast(n)]; + } + pub fn hasVulkan(self: *JniQuery) bool { + return labelle_android_has_system_feature(self.activity, "android.hardware.vulkan.version", vulkan_1_1) != 0; + } +}; + +/// Resolve the renderer and export `LABELLE_BGFX_RENDERER`. `activity` is the +/// running `ANativeActivity*` (opaque); `intent_extra` is this launch's +/// `LABELLE_BGFX_RENDERER` extra (null = absent). Comptime no-op off +/// Android; a null activity changes nothing. Called by `launch_intent.apply`, +/// after the intent extras are applied, so the value it writes is final. +pub fn resolve(activity: ?*const anyopaque, intent_extra: ?[:0]const u8) void { + if (comptime !is_android) return; + const a = activity orelse return; + var q: JniQuery = .{ .activity = a, .extra = intent_extra }; + const d = decide(&q); + if (setenv(env_name.ptr, d.renderer.envValue().ptr, 1) != 0) { + std.log.warn("android: could not set {s}={s}", .{ env_name, d.renderer.envValue() }); + } + std.log.info("renderer: {s} (source: {s})", .{ d.renderer.envValue(), d.source.label() }); +} + +// ── Tests ─────────────────────────────────────────────────────────────── + +const testing = std.testing; + +/// Scripted answers + a record of which questions were asked, so each test +/// asserts WHICH rule decided, not just the value. +const Fake = struct { + extra: ?[]const u8 = null, + is_debuggable: bool = false, + disabled: bool = false, + meta: ?[]const u8 = null, + vulkan: bool = false, + + debuggable_calls: usize = 0, + guard_calls: usize = 0, + meta_calls: usize = 0, + vulkan_calls: usize = 0, + + pub fn intentExtra(self: *Fake) ?[]const u8 { + return self.extra; + } + pub fn debuggable(self: *Fake) bool { + self.debuggable_calls += 1; + return self.is_debuggable; + } + pub fn vulkanDisabled(self: *Fake) bool { + self.guard_calls += 1; + return self.disabled; + } + pub fn metaData(self: *Fake) ?[]const u8 { + self.meta_calls += 1; + return self.meta; + } + pub fn hasVulkan(self: *Fake) bool { + self.vulkan_calls += 1; + return self.vulkan; + } +}; + +fn expectDecision(expected: Decision, got: Decision) !void { + try testing.expectEqual(expected.renderer, got.renderer); + try testing.expectEqual(expected.source, got.source); +} + +test "intent override wins in a debuggable apk; nothing later is asked" { + var f: Fake = .{ .extra = "vulkan", .is_debuggable = true, .disabled = true, .meta = "gles" }; + try expectDecision(.{ .renderer = .vulkan, .source = .intent }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.debuggable_calls); + try testing.expectEqual(@as(usize, 0), f.guard_calls); + try testing.expectEqual(@as(usize, 0), f.meta_calls); + + var g: Fake = .{ .extra = "gles", .is_debuggable = true, .meta = "vulkan" }; + try expectDecision(.{ .renderer = .gles, .source = .intent }, decide(&g)); + try testing.expectEqual(@as(usize, 0), g.meta_calls); +} + +test "the intent override is ignored when the apk is not debuggable" { + var f: Fake = .{ .extra = "vulkan", .is_debuggable = false, .meta = "gles" }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.debuggable_calls); + try testing.expectEqual(@as(usize, 1), f.meta_calls); +} + +test "an absent or empty extra never asks debuggable" { + var f: Fake = .{ .meta = "vulkan" }; + try expectDecision(.{ .renderer = .vulkan, .source = .setting }, decide(&f)); + var g: Fake = .{ .extra = "", .is_debuggable = true, .meta = "vulkan" }; + try expectDecision(.{ .renderer = .vulkan, .source = .setting }, decide(&g)); + try testing.expectEqual(@as(usize, 0), f.debuggable_calls + g.debuggable_calls); +} + +test "an invalid intent value falls through to the next rule" { + inline for (.{ "metal", "Vulkan", "opengl", "auto", " vulkan" }) |bad| { + var f: Fake = .{ .extra = bad, .is_debuggable = true, .meta = "vulkan" }; + try expectDecision(.{ .renderer = .vulkan, .source = .setting }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.meta_calls); + } +} + +test "the crash guard comes after the intent and before the setting" { + var f: Fake = .{ .disabled = true, .meta = "vulkan", .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .crash_guard }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.guard_calls); + try testing.expectEqual(@as(usize, 0), f.meta_calls); + try testing.expectEqual(@as(usize, 0), f.vulkan_calls); + + // A non-debuggable extra does not skip the guard either. + var g: Fake = .{ .extra = "vulkan", .disabled = true, .meta = "vulkan" }; + try expectDecision(.{ .renderer = .gles, .source = .crash_guard }, decide(&g)); +} + +test "the setting applies when there is no override and the guard is clear" { + var f: Fake = .{ .meta = "vulkan" }; + try expectDecision(.{ .renderer = .vulkan, .source = .setting }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.guard_calls); + try testing.expectEqual(@as(usize, 0), f.vulkan_calls); // explicit: no device check + var g: Fake = .{ .meta = "gles", .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&g)); +} + +test "missing meta-data means gles" { + var f: Fake = .{ .meta = null, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.meta_calls); + try testing.expectEqual(@as(usize, 0), f.vulkan_calls); +} + +test "an invalid meta-data value means gles" { + inline for (.{ "metal", "VULKAN", "Auto", "", "vulkan " }) |bad| { + var f: Fake = .{ .meta = bad, .vulkan = true }; + try expectDecision(.{ .renderer = .gles, .source = .setting }, decide(&f)); + try testing.expectEqual(@as(usize, 0), f.vulkan_calls); + } +} + +test "auto: Vulkan when the device has Vulkan 1.1, else gles" { + var f: Fake = .{ .meta = "auto", .vulkan = true }; + try expectDecision(.{ .renderer = .vulkan, .source = .auto }, decide(&f)); + try testing.expectEqual(@as(usize, 1), f.vulkan_calls); + var g: Fake = .{ .meta = "auto", .vulkan = false }; + try expectDecision(.{ .renderer = .gles, .source = .auto }, decide(&g)); + try testing.expectEqual(@as(usize, 1), g.vulkan_calls); +} + +test "env values and log labels" { + try testing.expectEqualStrings("vulkan", Renderer.vulkan.envValue()); + try testing.expectEqualStrings("gles", Renderer.gles.envValue()); + try testing.expectEqualStrings("crash-guard", Source.crash_guard.label()); + try testing.expectEqualStrings("intent", Source.intent.label()); + try testing.expectEqualStrings("setting", Source.setting.label()); + try testing.expectEqualStrings("auto", Source.auto.label()); +} + +test "resolve is a no-op off Android (and never touches the externs)" { + if (comptime is_android) return error.SkipZigTest; + resolve(null, null); + resolve(@ptrFromInt(0x1000), "vulkan"); +} diff --git a/src/root.zig b/src/root.zig index 38bf8ea..3238c03 100644 --- a/src/root.zig +++ b/src/root.zig @@ -16,6 +16,9 @@ //! * `video` — the MediaCodec H.264 decoder + audio-track decode //! (`VideoDecoder`, `decodeTrack`) and the pure `yuv`/`planes` helpers the //! desktop decoders share (FP#549; links `libmediandk`). +//! * `renderer` — resolve the renderer at launch and export +//! `LABELLE_BGFX_RENDERER` (labelle-android#27, labelle-bgfx#172); +//! `crash_guard` is its D11 seam (stub until labelle-android#28). //! //! Every file is reached from here by relative import inside this module. //! Consumers `@import("labelle_android")` and pass the running @@ -47,6 +50,8 @@ pub const relayout = @import("relayout.zig"); pub const aaudio = @import("aaudio.zig"); pub const video = @import("video.zig"); pub const immersive = @import("immersive.zig"); +pub const renderer = @import("renderer.zig"); +pub const crash_guard = @import("crash_guard.zig"); test { // Explicit refs: a lazily-referenced file's tests are otherwise never @@ -58,4 +63,6 @@ test { _ = aaudio; _ = video; _ = immersive; + _ = renderer; + _ = crash_guard; } From 2c48d4ea78f935f67432d21a971b4ea58fc2ba8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 01:16:37 -0300 Subject: [PATCH 2/8] =?UTF-8?q?feat:=20Vulkan=20crash=20guard=20=E2=80=94?= =?UTF-8?q?=20fall=20back=20to=20GLES=20after=20a=20crashed=20Vulkan=20sta?= =?UTF-8?q?rt=20(#28)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit labelle-bgfx#172 D11. Replaces #27's stub in src/crash_guard.zig: - .labelle_vulkan_start / .labelle_vulkan_disabled in internalDataPath, each " ". - beginVulkanStart (called by renderer.resolve when the result is vulkan) writes the start mark and spawns a detached thread that deletes it after 10 s (generation-checked, so a superseded timer leaves a newer mark). - A launch that finds the start mark writes the disabled mark; while the disabled stamp matches, vulkanDisabled() is true and the resolver picks gles, logging "renderer: gles (source: crash-guard; previous Vulkan start did not complete)". A new versionCode or setting (or a garbled stamp) deletes both files. - Pure checkAtLaunch/markStart/settle with injected fs + clock, host tests for every rule, incl. the intent override beating the guard. - JNI: labelle_android_version_code (getLongVersionCode, versionCode field fallback) + internal_data_path in renderer_query.c; failure → version 0, logged once. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- src/crash_guard.zig | 516 +++++++++++++++++++++++++++++++++++++-- src/jni/renderer_query.c | 80 ++++++ src/renderer.zig | 50 +++- src/root.zig | 2 +- 4 files changed, 625 insertions(+), 23 deletions(-) diff --git a/src/crash_guard.zig b/src/crash_guard.zig index 1e85509..6b99503 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -1,25 +1,507 @@ -//! Vulkan crash guard (labelle-bgfx#172 D11): the SEAM only. +//! Vulkan crash guard (labelle-android#28; labelle-bgfx#172 D11). //! -//! D11: mark "Vulkan start in progress" in the app's internal storage before a -//! Vulkan init, clear it once the game has run stably, and start on GLES at -//! the next launch if the mark is still there. That work is -//! labelle-toolkit/labelle-android#28. Until it lands this is a stub that -//! never disables Vulkan, so `renderer.resolve` (#27) already consults the -//! guard in the right place (after the intent override, before the provider -//! setting) and #28 only has to replace the body below. +//! If the game crashes (or is killed) while starting on Vulkan, the next +//! launch starts on GLES, so a bad Vulkan driver cannot brick the game. +//! +//! Two marker files in the app's internal data dir +//! (`ANativeActivity.internalDataPath`), each containing +//! ` `: +//! +//! * `.labelle_vulkan_start`: written by `beginVulkanStart`, just before a +//! Vulkan start (the resolved renderer is `vulkan`). +//! * `.labelle_vulkan_disabled`: written when a launch finds +//! `.labelle_vulkan_start` still there, i.e. the previous Vulkan start +//! never reached "stable". +//! +//! Rules: +//! * Stable: a detached thread deletes `.labelle_vulkan_start` once the +//! process has lived `stable_after_ns` (10 s) after `beginVulkanStart`. +//! No labelle-bgfx signal (the generic-only rule). +//! * Disabled: while `.labelle_vulkan_disabled` exists and its stamp +//! matches the current `versionCode` and setting, `vulkanDisabled` +//! returns true and `renderer.decide` picks `gles` (source +//! `crash-guard`). +//! * Reset: a stamp that does NOT match (a new app version or a changed +//! `renderer` setting) deletes both files, so Vulkan is tried again. An +//! unreadable stamp resets too. +//! * Intent override: `renderer.decide` asks the debuggable-only intent +//! extra BEFORE the guard, so it wins (see the test at the bottom). +//! +//! The PURE half (`checkAtLaunch`, `markStart`, `settle`) takes an injected +//! filesystem and clock and is host-tested; the Android glue below it uses +//! libc and the JNI helpers in `jni/renderer_query.c`. const std = @import("std"); +const is_android = @import("root.zig").is_android; + +pub const start_file = ".labelle_vulkan_start"; +pub const disabled_file = ".labelle_vulkan_disabled"; +/// How long the process must live after `beginVulkanStart` before the start +/// counts as stable. +pub const stable_after_ns: u64 = 10 * std.time.ns_per_s; + +/// What both marker files contain: ` `. +pub const Stamp = struct { + version_code: i64, + /// The effective provider setting (`gles`, `vulkan` or `auto`). + setting: []const u8, + + pub fn eql(a: Stamp, b: Stamp) bool { + return a.version_code == b.version_code and std.mem.eql(u8, a.setting, b.setting); + } + + /// ` `, or null if it does not fit `buf`. + pub fn format(self: Stamp, buf: []u8) ?[]const u8 { + return std.fmt.bufPrint(buf, "{d} {s}", .{ self.version_code, self.setting }) catch null; + } + + /// Parse ` ` (trailing whitespace allowed). Null + /// for anything else; `setting` borrows from `bytes`. + pub fn parse(bytes: []const u8) ?Stamp { + const trimmed = std.mem.trimEnd(u8, bytes, " \t\r\n"); + const sp = std.mem.indexOfScalar(u8, trimmed, ' ') orelse return null; + const version_code = std.fmt.parseInt(i64, trimmed[0..sp], 10) catch return null; + const setting = trimmed[sp + 1 ..]; + if (setting.len == 0 or std.mem.indexOfAny(u8, setting, " \t\r\n") != null) return null; + return .{ .version_code = version_code, .setting = setting }; + } +}; + +/// Room for a stamp: a 20-digit i64, a space and the longest setting. +const stamp_cap = 64; -/// True when a previous launch crashed during a Vulkan start and this launch -/// must use GLES instead. `activity` is the running `ANativeActivity*` -/// (opaque), for the internal-storage path #28 will need. +/// The launch-time check. `fs` provides: +/// * `read(name: []const u8, buf: []u8) ?[]const u8` (null = absent or +/// unreadable) +/// * `write(name: []const u8, bytes: []const u8) bool` +/// * `delete(name: []const u8) void` (absent is fine) /// -/// Stub: always "not disabled". -pub fn isVulkanDisabled(activity: ?*const anyopaque) bool { - _ = activity; +/// Returns true when this launch must not use Vulkan. Side effects, in order: +/// 1. `.labelle_vulkan_disabled` present: matching stamp → true (and a +/// leftover start mark is dropped); otherwise reset (both files go). +/// 2. `.labelle_vulkan_start` present (the previous Vulkan start did not +/// complete): matching stamp → write `.labelle_vulkan_disabled`, drop the +/// start mark, true; otherwise reset. +/// 3. Neither → false. +pub fn checkAtLaunch(fs: anytype, current: Stamp) bool { + var buf: [stamp_cap]u8 = undefined; + if (fs.read(disabled_file, &buf)) |bytes| { + if (Stamp.parse(bytes)) |s| if (s.eql(current)) { + fs.delete(start_file); + return true; + }; + reset(fs); + return false; + } + if (fs.read(start_file, &buf)) |bytes| { + if (Stamp.parse(bytes)) |s| if (s.eql(current)) { + var out: [stamp_cap]u8 = undefined; + if (current.format(&out)) |text| { + if (!fs.write(disabled_file, text)) { + std.log.warn("android: crash guard: could not write {s}", .{disabled_file}); + } + } + fs.delete(start_file); + // Disabled for THIS launch even if the write failed: the crash + // just happened, so do not retry Vulkan right away. + return true; + }; + reset(fs); + return false; + } return false; } -test "stub: Vulkan is never disabled" { - try std.testing.expect(!isVulkanDisabled(null)); - try std.testing.expect(!isVulkanDisabled(@ptrFromInt(0x1000))); +fn reset(fs: anytype) void { + fs.delete(start_file); + fs.delete(disabled_file); +} + +/// Write `.labelle_vulkan_start` (called just before a Vulkan start). +pub fn markStart(fs: anytype, current: Stamp) bool { + var out: [stamp_cap]u8 = undefined; + const text = current.format(&out) orelse return false; + return fs.write(start_file, text); +} + +/// The stable timer: sleep `stable_after_ns`, then delete the start mark, +/// unless a newer `beginVulkanStart` (another activity launch in the same +/// process) has taken over (`generation` moved past `mine`); its own timer +/// will clear the mark. `clock.sleep(ns) !void` — a fake returns an error to +/// model the process dying mid-sleep. +pub fn settle(fs: anytype, clock: anytype, generation: *const std.atomic.Value(u32), mine: u32) void { + clock.sleep(stable_after_ns) catch return; + if (generation.load(.acquire) != mine) return; + fs.delete(start_file); +} + +// ── Android glue ──────────────────────────────────────────────────────── + +// `jni/renderer_query.c`. +extern "c" fn labelle_android_internal_data_path(activity: ?*const anyopaque) ?[*:0]const u8; +extern "c" fn labelle_android_version_code(activity: ?*const anyopaque, out: *c_longlong) c_int; +// libc. +const FILE = opaque {}; +extern "c" fn fopen(name: [*:0]const u8, mode: [*:0]const u8) ?*FILE; +extern "c" fn fread(ptr: [*]u8, size: usize, n: usize, f: *FILE) usize; +extern "c" fn fwrite(ptr: [*]const u8, size: usize, n: usize, f: *FILE) usize; +extern "c" fn fclose(f: *FILE) c_int; +extern "c" fn unlink(name: [*:0]const u8) c_int; +extern "c" fn sleep(seconds: c_uint) c_uint; + +/// Marker files under a copied `internalDataPath` (a value type, so the +/// detached stable thread owns its own copy). +const LibcFs = struct { + dir_buf: [512]u8 = undefined, + dir_len: usize = 0, + + fn init(dir: []const u8) ?LibcFs { + var self: LibcFs = .{}; + if (dir.len == 0 or dir.len > self.dir_buf.len) return null; + @memcpy(self.dir_buf[0..dir.len], dir); + self.dir_len = dir.len; + return self; + } + + fn full(self: *const LibcFs, name: []const u8, buf: []u8) ?[:0]const u8 { + return std.fmt.bufPrintZ(buf, "{s}/{s}", .{ self.dir_buf[0..self.dir_len], name }) catch null; + } + + pub fn read(self: *const LibcFs, name: []const u8, buf: []u8) ?[]const u8 { + var p: [600]u8 = undefined; + const f = fopen((self.full(name, &p) orelse return null).ptr, "rb") orelse return null; + defer _ = fclose(f); + const n = fread(buf.ptr, 1, buf.len, f); + return buf[0..n]; + } + pub fn write(self: *const LibcFs, name: []const u8, bytes: []const u8) bool { + var p: [600]u8 = undefined; + const f = fopen((self.full(name, &p) orelse return false).ptr, "wb") orelse return false; + const ok = fwrite(bytes.ptr, 1, bytes.len, f) == bytes.len; + return fclose(f) == 0 and ok; + } + pub fn delete(self: *const LibcFs, name: []const u8) void { + var p: [600]u8 = undefined; + _ = unlink((self.full(name, &p) orelse return).ptr); + } +}; + +const LibcClock = struct { + pub fn sleep(_: LibcClock, ns: u64) error{}!void { + var left: c_uint = @intCast(std.math.divCeil(u64, ns, std.time.ns_per_s) catch unreachable); + // `sleep` returns the unslept seconds when a signal interrupts it. + while (left > 0) left = sleep_c(left); + } +}; +const sleep_c = sleep; + +/// Bumped by every `beginVulkanStart`; each stable thread only clears the +/// mark if it is still the latest. +var start_generation: std.atomic.Value(u32) = .init(0); + +/// Process-wide `versionCode` cache: it cannot change for the life of the +/// process (an update kills it). -1 = not asked yet. +var version_cache: std.atomic.Value(i64) = .init(-1); + +fn versionCode(activity: *const anyopaque) i64 { + const cached = version_cache.load(.acquire); + if (cached >= 0) return cached; + var v: c_longlong = 0; + const got: i64 = if (labelle_android_version_code(activity, &v) == 0 and v >= 0) v else 0; + // First store wins; only the winner logs, so the failure is logged once. + if (version_cache.cmpxchgStrong(-1, got, .acq_rel, .acquire)) |theirs| return theirs; + if (got == 0) std.log.warn("android: crash guard: could not read the app's versionCode; using 0", .{}); + return got; +} + +fn libcFs(activity: *const anyopaque) ?LibcFs { + const dir = labelle_android_internal_data_path(activity) orelse { + std.log.warn("android: crash guard: no internalDataPath; guard off", .{}); + return null; + }; + return LibcFs.init(std.mem.span(dir)) orelse { + std.log.warn("android: crash guard: internalDataPath unusable; guard off", .{}); + return null; + }; +} + +/// True when a previous launch crashed during a Vulkan start (and the app +/// version and `setting` are unchanged since), so this launch must use GLES. +/// Also applies the reset rule. `activity` is the running `ANativeActivity*` +/// (opaque); `setting` the effective provider setting (`gles`|`vulkan`| +/// `auto`). Off Android, or with no activity or data dir: false. +pub fn vulkanDisabled(activity: ?*const anyopaque, setting: []const u8) bool { + if (comptime !is_android) return false; + const a = activity orelse return false; + var fs = libcFs(a) orelse return false; + return checkAtLaunch(&fs, .{ .version_code = versionCode(a), .setting = setting }); +} + +/// Called by `renderer.resolve` once Vulkan is chosen, before bgfx starts: +/// write `.labelle_vulkan_start` and start the detached 10 s stable timer. +pub fn beginVulkanStart(activity: ?*const anyopaque, setting: []const u8) void { + if (comptime !is_android) return; + const a = activity orelse return; + const fs = libcFs(a) orelse return; + const mine = start_generation.fetchAdd(1, .acq_rel) + 1; + if (!markStart(&fs, .{ .version_code = versionCode(a), .setting = setting })) { + std.log.warn("android: crash guard: could not write {s}", .{start_file}); + return; + } + const t = std.Thread.spawn(.{}, stableThread, .{ fs, mine }) catch |err| { + // No timer: the mark stays and the NEXT launch uses GLES. Safer to + // drop the mark now and run unguarded than to disable Vulkan for a + // start that may well succeed. + std.log.warn("android: crash guard: could not start the stable timer ({s}); guard off for this launch", .{@errorName(err)}); + fs.delete(start_file); + return; + }; + t.detach(); +} + +fn stableThread(fs: LibcFs, mine: u32) void { + settle(&fs, LibcClock{}, &start_generation, mine); +} + +// ── Tests ─────────────────────────────────────────────────────────────── + +const testing = std.testing; + +/// In-memory marker files, persisting across "launches" like internal +/// storage does. +const FakeFs = struct { + start: ?[]const u8 = null, + disabled: ?[]const u8 = null, + start_buf: [stamp_cap]u8 = undefined, + disabled_buf: [stamp_cap]u8 = undefined, + fail_writes: bool = false, + + fn slot(self: *FakeFs, name: []const u8) struct { *?[]const u8, []u8 } { + if (std.mem.eql(u8, name, start_file)) return .{ &self.start, &self.start_buf }; + if (std.mem.eql(u8, name, disabled_file)) return .{ &self.disabled, &self.disabled_buf }; + @panic("unexpected file name"); + } + pub fn read(self: *FakeFs, name: []const u8, buf: []u8) ?[]const u8 { + const s = self.slot(name); + const v = s[0].* orelse return null; + @memcpy(buf[0..v.len], v); + return buf[0..v.len]; + } + pub fn write(self: *FakeFs, name: []const u8, bytes: []const u8) bool { + if (self.fail_writes) return false; + const s = self.slot(name); + @memcpy(s[1][0..bytes.len], bytes); + s[0].* = s[1][0..bytes.len]; + return true; + } + pub fn delete(self: *FakeFs, name: []const u8) void { + self.slot(name)[0].* = null; + } + fn set(self: *FakeFs, name: []const u8, bytes: []const u8) void { + const saved = self.fail_writes; + self.fail_writes = false; + _ = self.write(name, bytes); + self.fail_writes = saved; + } +}; + +/// `sleep` advances a fake clock; the process "dies" at `dies_at_ns`. +const FakeClock = struct { + now_ns: u64 = 0, + dies_at_ns: ?u64 = null, + slept_ns: u64 = 0, + + pub fn sleep(self: *FakeClock, ns: u64) error{ProcessDied}!void { + if (self.dies_at_ns) |d| if (self.now_ns + ns >= d) { + self.now_ns = d; + return error.ProcessDied; + }; + self.now_ns += ns; + self.slept_ns += ns; + } +}; + +const v1_vulkan: Stamp = .{ .version_code = 1, .setting = "vulkan" }; + +/// One Vulkan launch: check the guard; if clear, mark the start and run the +/// stable timer on `clock`. +fn launchVulkan(fs: *FakeFs, clock: *FakeClock, gen: *std.atomic.Value(u32), stamp: Stamp) bool { + if (checkAtLaunch(fs, stamp)) return false; + const mine = gen.fetchAdd(1, .acq_rel) + 1; + if (!markStart(fs, stamp)) return true; + settle(fs, clock, gen, mine); + return true; +} + +test "stamp: format and parse round-trip; junk is rejected" { + var buf: [stamp_cap]u8 = undefined; + const text = v1_vulkan.format(&buf).?; + try testing.expectEqualStrings("1 vulkan", text); + try testing.expect(Stamp.parse(text).?.eql(v1_vulkan)); + try testing.expect(Stamp.parse("42 auto\n").?.eql(.{ .version_code = 42, .setting = "auto" })); + try testing.expect(Stamp.parse("9223372036854775807 gles").?.version_code == std.math.maxInt(i64)); + inline for (.{ "", "1", "1 ", "x vulkan", " 1 vulkan", "1 vul kan", "1\tvulkan" }) |bad| { + try testing.expect(Stamp.parse(bad) == null); + } +} + +test "first start: nothing on disk, Vulkan allowed, start mark written" { + var fs: FakeFs = .{}; + try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.start == null and fs.disabled == null); + try testing.expect(markStart(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.start.?); +} + +test "a crash inside 10 s disables Vulkan at the next launch" { + var fs: FakeFs = .{}; + var gen: std.atomic.Value(u32) = .init(0); + var clock: FakeClock = .{ .dies_at_ns = stable_after_ns - 1 }; + try testing.expect(launchVulkan(&fs, &clock, &gen, v1_vulkan)); + // The timer never finished, so the mark survived the "crash". + try testing.expectEqual(@as(u64, 0), clock.slept_ns); + try testing.expectEqualStrings("1 vulkan", fs.start.?); + + // Next launch: the stale start becomes the disabled mark. + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.start == null); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); + // ...and it stays disabled on every later launch with the same stamp. + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); +} + +test "a failed disabled write still keeps THIS launch off Vulkan" { + var fs: FakeFs = .{}; + fs.set(start_file, "1 vulkan"); + fs.fail_writes = true; + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.start == null and fs.disabled == null); +} + +test "surviving 10 s clears the mark; the next launch tries Vulkan again" { + var fs: FakeFs = .{}; + var gen: std.atomic.Value(u32) = .init(0); + var clock: FakeClock = .{}; + try testing.expect(launchVulkan(&fs, &clock, &gen, v1_vulkan)); + try testing.expectEqual(stable_after_ns, clock.slept_ns); + try testing.expect(fs.start == null and fs.disabled == null); + try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.disabled == null); +} + +test "a superseded stable timer leaves the newer start mark alone" { + var fs: FakeFs = .{}; + var gen: std.atomic.Value(u32) = .init(0); + const first = gen.fetchAdd(1, .acq_rel) + 1; + try testing.expect(markStart(&fs, v1_vulkan)); + _ = gen.fetchAdd(1, .acq_rel); // a second beginVulkanStart in the same process + var clock: FakeClock = .{}; + settle(&fs, &clock, &gen, first); + try testing.expectEqual(stable_after_ns, clock.slept_ns); // it did sleep... + try testing.expect(fs.start != null); // ...but did not delete +} + +test "a version bump resets both files and Vulkan is tried again" { + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + fs.set(start_file, "1 vulkan"); + const v2: Stamp = .{ .version_code = 2, .setting = "vulkan" }; + try testing.expect(!checkAtLaunch(&fs, v2)); + try testing.expect(fs.start == null and fs.disabled == null); + + // Same for a stale start mark alone (crashed on v1, launched on v2). + fs.set(start_file, "1 vulkan"); + try testing.expect(!checkAtLaunch(&fs, v2)); + try testing.expect(fs.start == null and fs.disabled == null); +} + +test "a changed renderer setting resets both files" { + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + fs.set(start_file, "1 vulkan"); + try testing.expect(!checkAtLaunch(&fs, .{ .version_code = 1, .setting = "auto" })); + try testing.expect(fs.start == null and fs.disabled == null); + + fs.set(start_file, "1 auto"); + try testing.expect(!checkAtLaunch(&fs, .{ .version_code = 1, .setting = "gles" })); + try testing.expect(fs.start == null and fs.disabled == null); +} + +test "an unreadable stamp resets" { + var fs: FakeFs = .{}; + fs.set(disabled_file, "garbage"); + try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.disabled == null); + fs.set(start_file, ""); + try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.start == null); +} + +test "a disabled launch drops a leftover start mark" { + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + fs.set(start_file, "1 vulkan"); + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.start == null); + try testing.expect(fs.disabled != null); +} + +test "an intent override beats the guard (renderer.decide asks the intent first)" { + const renderer = @import("renderer.zig"); + const Q = struct { + fs: *FakeFs, + extra: ?[]const u8, + is_debuggable: bool, + guard_calls: usize = 0, + pub fn intentExtra(self: *@This()) ?[]const u8 { + return self.extra; + } + pub fn debuggable(self: *@This()) bool { + return self.is_debuggable; + } + pub fn vulkanDisabled(self: *@This()) bool { + self.guard_calls += 1; + return checkAtLaunch(self.fs, v1_vulkan); + } + pub fn metaData(_: *@This()) ?[]const u8 { + return "vulkan"; + } + pub fn hasVulkan(_: *@This()) bool { + return true; + } + }; + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + + // Debuggable + extra: Vulkan from the intent; the guard is not consulted. + var q: Q = .{ .fs = &fs, .extra = "vulkan", .is_debuggable = true }; + const d = renderer.decide(&q); + try testing.expectEqual(renderer.Renderer.vulkan, d.renderer); + try testing.expectEqual(renderer.Source.intent, d.source); + try testing.expectEqual(@as(usize, 0), q.guard_calls); + + // Not debuggable: the extra is ignored and the guard wins. + var r: Q = .{ .fs = &fs, .extra = "vulkan", .is_debuggable = false }; + const e = renderer.decide(&r); + try testing.expectEqual(renderer.Renderer.gles, e.renderer); + try testing.expectEqual(renderer.Source.crash_guard, e.source); + try testing.expectEqual(@as(usize, 1), r.guard_calls); +} + +test "the Android entry points are no-ops off Android" { + if (comptime is_android) return error.SkipZigTest; + try testing.expect(!vulkanDisabled(null, "vulkan")); + try testing.expect(!vulkanDisabled(@ptrFromInt(0x1000), "vulkan")); + beginVulkanStart(@ptrFromInt(0x1000), "vulkan"); +} + +test "Android: the glue is analysed (compile-check only; nothing runs)" { + if (comptime !is_android) return error.SkipZigTest; + // Address-of forces semantic analysis of the Android branches, their + // externs and the JNI helpers' signatures in the NDK compile-check. + _ = &vulkanDisabled; + _ = &beginVulkanStart; + _ = &stableThread; + _ = &@import("renderer.zig").resolve; } diff --git a/src/jni/renderer_query.c b/src/jni/renderer_query.c index 6fd9174..b728281 100644 --- a/src/jni/renderer_query.c +++ b/src/jni/renderer_query.c @@ -11,6 +11,8 @@ // * `auto`'s device check: // getPackageManager().hasSystemFeature("android.hardware.vulkan.version", // 0x401000) // Vulkan 1.1 +// * for the crash guard (labelle-android#28): the package's version code +// and `ANativeActivity.internalDataPath` // // In C for the same reason as intent_extras.c: already declares the // JNI vtables. Off Android this is an empty TU. @@ -155,4 +157,82 @@ int labelle_android_has_system_feature(const void *activity_ptr, const char *nam return has; } +// `ANativeActivity.internalDataPath` (the app's private files dir), or NULL. +// The crash guard (labelle-android#28) keeps its two marker files there. +const char *labelle_android_internal_data_path(const void *activity_ptr) { + const ANativeActivity *na = (const ANativeActivity *)activity_ptr; + return na != NULL ? na->internalDataPath : NULL; +} + +// The running package's version code (labelle-android#28: a new app version +// resets the crash guard): +// getPackageManager().getPackageInfo(getPackageName(), 0) +// .getLongVersionCode() // API 28+ +// .versionCode // older: NoSuchMethodError -> the int field +// Writes it to `*out` and returns 0, or returns -1 on any JNI failure +// (`*out` untouched). Never leaves a Java exception pending and never leaks a +// local ref. Callable from any thread (attached or not). +int labelle_android_version_code(const void *activity_ptr, long long *out) { + const ANativeActivity *na = (const ANativeActivity *)activity_ptr; + if (na == NULL || na->vm == NULL || na->clazz == NULL || out == NULL) return -1; + JavaVM *vm = na->vm; + jobject activity = na->clazz; + + int we_attached = 0; + JNIEnv *env = labelle_android_acquire_env(vm, &we_attached); + if (env == NULL) return -1; + + int result = -1; + if ((*env)->PushLocalFrame(env, 16) == JNI_OK) { + jobject pm = package_manager(env, activity); + jclass activity_cls = (pm != NULL && !(*env)->ExceptionCheck(env)) ? (*env)->GetObjectClass(env, activity) : NULL; + jmethodID get_pkg = activity_cls ? (*env)->GetMethodID(env, activity_cls, "getPackageName", "()Ljava/lang/String;") : NULL; + jstring pkg = (get_pkg && !(*env)->ExceptionCheck(env)) ? (jstring)(*env)->CallObjectMethod(env, activity, get_pkg) : NULL; + jclass pm_cls = (pkg && !(*env)->ExceptionCheck(env)) ? (*env)->GetObjectClass(env, pm) : NULL; + // The (String, int) overload: deprecated from API 33 in favour of + // the PackageInfoFlags one, but still present and working. + jmethodID get_pkg_info = pm_cls ? (*env)->GetMethodID(env, pm_cls, "getPackageInfo", "(Ljava/lang/String;I)Landroid/content/pm/PackageInfo;") : NULL; + // Throws NameNotFoundException only if our own package vanished. + jobject pkg_info = (get_pkg_info && !(*env)->ExceptionCheck(env)) ? (*env)->CallObjectMethod(env, pm, get_pkg_info, pkg, (jint)0) : NULL; + if (pkg_info != NULL && !(*env)->ExceptionCheck(env)) { + jclass pi_cls = (*env)->GetObjectClass(env, pkg_info); + if (pi_cls != NULL && !(*env)->ExceptionCheck(env)) { + jmethodID get_long = (*env)->GetMethodID(env, pi_cls, "getLongVersionCode", "()J"); + if (get_long != NULL && !(*env)->ExceptionCheck(env)) { + jlong v = (*env)->CallLongMethod(env, pkg_info, get_long); + if (!(*env)->ExceptionCheck(env)) { + *out = (long long)v; + result = 0; + } + } else { + // Below API 28: NoSuchMethodError. Clear it and read the + // (deprecated) int field instead. + if ((*env)->ExceptionCheck(env)) (*env)->ExceptionClear(env); + jfieldID vc_fid = (*env)->GetFieldID(env, pi_cls, "versionCode", "I"); + if (vc_fid != NULL && !(*env)->ExceptionCheck(env)) { + jint v = (*env)->GetIntField(env, pkg_info, vc_fid); + if (!(*env)->ExceptionCheck(env)) { + *out = (long long)v; + result = 0; + } + } + } + } + } + // Any JNI call above may have raised; clear before popping so we + // never hand a pending exception back to the caller's thread. + if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + result = -1; + } + (*env)->PopLocalFrame(env, NULL); + } else if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + __android_log_print(ANDROID_LOG_WARN, "labelle-android", "version code: PushLocalFrame failed; exception cleared"); + } + + if (we_attached) (*vm)->DetachCurrentThread(vm); + return result; +} + #endif /* __ANDROID__ */ diff --git a/src/renderer.zig b/src/renderer.zig index 8916edd..e93d4f9 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -10,7 +10,9 @@ //! only in a debuggable apk (the same gate as the other debuggable-only //! keys in `intent_env.zig`). Anything else is warned about and ignored. //! 2. `crash-guard`: Vulkan disabled after a crashed Vulkan start (D11, -//! `crash_guard.zig`; a stub until labelle-android#28) → `gles`. +//! `crash_guard.zig`, labelle-android#28) → `gles`. The guard is keyed +//! on the app's versionCode and the effective setting (rule 3), so a +//! new version or a changed setting tries Vulkan again. //! 3. `setting`: the provider setting, stamped into the manifest as //! `` //! (labelle-android#26). Missing or unreadable → `gles`; an invalid @@ -18,8 +20,11 @@ //! 4. `auto` (the setting's third value): Vulkan when the device reports //! `android.hardware.vulkan.version` >= 1.1 (0x401000), else `gles`. //! -//! Then `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles", 1)` and one log -//! line: `renderer: (source: intent|crash-guard|setting|auto)`. +//! Then, when the result is `vulkan`, `crash_guard.beginVulkanStart` (the +//! start mark + the 10 s stable timer); `setenv("LABELLE_BGFX_RENDERER", +//! "vulkan"|"gles", 1)`; and one log line: +//! `renderer: (source: intent|crash-guard|setting|auto)`, with +//! `; previous Vulkan start did not complete` after `crash-guard`. //! //! ## Where it runs //! @@ -71,6 +76,14 @@ pub const Source = enum { .auto => "auto", }; } + + /// Extra context after the label in the log line ("" for none). + pub fn detail(self: Source) []const u8 { + return switch (self) { + .crash_guard => "; previous Vulkan start did not complete", + else => "", + }; + } }; pub const Decision = struct { renderer: Renderer, source: Source }; @@ -78,6 +91,13 @@ pub const Decision = struct { renderer: Renderer, source: Source }; /// The provider setting's values. pub const Setting = enum { gles, vulkan, auto }; +/// The effective setting the crash guard's stamp records: the parsed +/// meta-data, and `gles` for missing or invalid (which is what they mean). +pub fn settingLabel(meta: ?[]const u8) []const u8 { + const raw = meta orelse return "gles"; + return @tagName(parseSetting(raw) orelse .gles); +} + /// An explicit renderer (the intent extra). Exact, lower-case match only. pub fn parseRenderer(value: []const u8) ?Renderer { if (std.mem.eql(u8, value, "vulkan")) return .vulkan; @@ -144,6 +164,9 @@ const JniQuery = struct { activity: *const anyopaque, extra: ?[:0]const u8, meta_buf: [64]u8 = undefined, + /// The meta-data is asked for by the guard (for its stamp) and by the + /// setting rule; the JNI walk (and its log line) happens once. + meta: ?(?[]const u8) = null, pub fn intentExtra(self: *JniQuery) ?[]const u8 { return self.extra; @@ -152,9 +175,15 @@ const JniQuery = struct { return debuggable_mod.isDebuggable(self.activity); } pub fn vulkanDisabled(self: *JniQuery) bool { - return crash_guard.isVulkanDisabled(self.activity); + return crash_guard.vulkanDisabled(self.activity, settingLabel(self.metaData())); } pub fn metaData(self: *JniQuery) ?[]const u8 { + if (self.meta) |m| return m; + const m = self.readMeta(); + self.meta = m; + return m; + } + fn readMeta(self: *JniQuery) ?[]const u8 { const n = labelle_android_read_renderer_meta(self.activity, &self.meta_buf, self.meta_buf.len); switch (n) { -1 => { @@ -187,10 +216,11 @@ pub fn resolve(activity: ?*const anyopaque, intent_extra: ?[:0]const u8) void { const a = activity orelse return; var q: JniQuery = .{ .activity = a, .extra = intent_extra }; const d = decide(&q); + if (d.renderer == .vulkan) crash_guard.beginVulkanStart(a, settingLabel(q.metaData())); if (setenv(env_name.ptr, d.renderer.envValue().ptr, 1) != 0) { std.log.warn("android: could not set {s}={s}", .{ env_name, d.renderer.envValue() }); } - std.log.info("renderer: {s} (source: {s})", .{ d.renderer.envValue(), d.source.label() }); + std.log.info("renderer: {s} (source: {s}{s})", .{ d.renderer.envValue(), d.source.label(), d.source.detail() }); } // ── Tests ─────────────────────────────────────────────────────────────── @@ -324,6 +354,16 @@ test "env values and log labels" { try testing.expectEqualStrings("intent", Source.intent.label()); try testing.expectEqualStrings("setting", Source.setting.label()); try testing.expectEqualStrings("auto", Source.auto.label()); + try testing.expectEqualStrings("; previous Vulkan start did not complete", Source.crash_guard.detail()); + try testing.expectEqualStrings("", Source.setting.detail()); +} + +test "settingLabel: the effective setting for the crash guard's stamp" { + try testing.expectEqualStrings("vulkan", settingLabel("vulkan")); + try testing.expectEqualStrings("auto", settingLabel("auto")); + try testing.expectEqualStrings("gles", settingLabel("gles")); + try testing.expectEqualStrings("gles", settingLabel(null)); + try testing.expectEqualStrings("gles", settingLabel("metal")); } test "resolve is a no-op off Android (and never touches the externs)" { diff --git a/src/root.zig b/src/root.zig index 3238c03..03152f1 100644 --- a/src/root.zig +++ b/src/root.zig @@ -18,7 +18,7 @@ //! desktop decoders share (FP#549; links `libmediandk`). //! * `renderer` — resolve the renderer at launch and export //! `LABELLE_BGFX_RENDERER` (labelle-android#27, labelle-bgfx#172); -//! `crash_guard` is its D11 seam (stub until labelle-android#28). +//! `crash_guard` is its D11 Vulkan crash guard (labelle-android#28). //! //! Every file is reached from here by relative import inside this module. //! Consumers `@import("labelle_android")` and pass the running From 39d02048c53b6b4b989d68e8eb64f0e1e3f11a20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 01:37:54 -0300 Subject: [PATCH 3/8] fix(crash_guard): fail closed; guard active in debuggable builds (#28 review) Codex review of #33 (labelle-bgfx#172 comment 5904099277): - A failed .labelle_vulkan_disabled write keeps the start mark, so the next launch is still guarded; a stable timer that cannot be spawned keeps the mark too (pure beginStart with an injected spawner, host-tested). - Host test: a debuggable build WITHOUT the intent extra is still guarded. - Doc: an early force-stop (am start -S inside the window) counts as an incomplete start by design; bypass with --es LABELLE_BGFX_RENDERER vulkan. - TODO(#172 D11) on the 10 s timer: it starts before bgfx.init, so a hung Vulkan init past 10 s clears the mark; needs a lifecycle signal. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- src/crash_guard.zig | 135 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 109 insertions(+), 26 deletions(-) diff --git a/src/crash_guard.zig b/src/crash_guard.zig index 6b99503..2aae5e5 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -17,6 +17,11 @@ //! * Stable: a detached thread deletes `.labelle_vulkan_start` once the //! process has lived `stable_after_ns` (10 s) after `beginVulkanStart`. //! No labelle-bgfx signal (the generic-only rule). +//! TODO(#172 D11): the timer starts at `renderer.resolve`, BEFORE +//! `bgfx.init`, so a Vulkan init that hangs for more than 10 s clears the +//! mark and is not caught (Codex review of labelle-android#33, +//! labelle-bgfx#172 comment 5904099277). The fix needs a lifecycle +//! signal; waiting on an owner decision. //! * Disabled: while `.labelle_vulkan_disabled` exists and its stamp //! matches the current `versionCode` and setting, `vulkanDisabled` //! returns true and `renderer.decide` picks `gles` (source @@ -25,7 +30,18 @@ //! `renderer` setting) deletes both files, so Vulkan is tried again. An //! unreadable stamp resets too. //! * Intent override: `renderer.decide` asks the debuggable-only intent -//! extra BEFORE the guard, so it wins (see the test at the bottom). +//! extra BEFORE the guard, so it wins (see the test at the bottom). The +//! guard itself stays ACTIVE in debuggable builds: only an explicit +//! extra bypasses it. +//! * Fails CLOSED: if the disabled mark cannot be written, or the stable +//! timer cannot be started, the start mark is KEPT, so the next launch +//! is still guarded (it finds the mark again and goes to GLES). +//! +//! By design, anything that ends the process inside the stable window counts +//! as an incomplete start, including a developer's early force-stop (e.g. +//! `am start -S` within 10 s of the previous launch): the next launch runs +//! on GLES. To bypass the guard while iterating, launch a debuggable build +//! with `--es LABELLE_BGFX_RENDERER vulkan` (the intent override). //! //! The PURE half (`checkAtLaunch`, `markStart`, `settle`) takes an injected //! filesystem and clock and is host-tested; the Android glue below it uses @@ -80,7 +96,9 @@ const stamp_cap = 64; /// leftover start mark is dropped); otherwise reset (both files go). /// 2. `.labelle_vulkan_start` present (the previous Vulkan start did not /// complete): matching stamp → write `.labelle_vulkan_disabled`, drop the -/// start mark, true; otherwise reset. +/// start mark, true; otherwise reset. If the write fails, the start +/// mark is KEPT (fail closed): the next launch finds it again and is +/// still guarded. /// 3. Neither → false. pub fn checkAtLaunch(fs: anytype, current: Stamp) bool { var buf: [stamp_cap]u8 = undefined; @@ -95,14 +113,14 @@ pub fn checkAtLaunch(fs: anytype, current: Stamp) bool { if (fs.read(start_file, &buf)) |bytes| { if (Stamp.parse(bytes)) |s| if (s.eql(current)) { var out: [stamp_cap]u8 = undefined; - if (current.format(&out)) |text| { - if (!fs.write(disabled_file, text)) { - std.log.warn("android: crash guard: could not write {s}", .{disabled_file}); - } + const written = if (current.format(&out)) |text| fs.write(disabled_file, text) else false; + if (written) { + fs.delete(start_file); + } else { + // Fail closed: keep the start mark so the NEXT launch is + // still guarded. This launch is off Vulkan either way. + std.log.warn("android: crash guard: could not write {s}; keeping {s}", .{ disabled_file, start_file }); } - fs.delete(start_file); - // Disabled for THIS launch even if the write failed: the crash - // just happened, so do not retry Vulkan right away. return true; }; reset(fs); @@ -123,10 +141,29 @@ pub fn markStart(fs: anytype, current: Stamp) bool { return fs.write(start_file, text); } +/// `beginVulkanStart`'s pure half: bump the generation, write the start mark, +/// start the stable timer via `spawner.spawn(mine: u32) !void`. Fails +/// CLOSED: if the timer cannot start, the mark is KEPT, so the next launch +/// runs on GLES rather than an unguarded Vulkan start being forgotten. +pub fn beginStart(fs: anytype, current: Stamp, generation: *std.atomic.Value(u32), spawner: anytype) void { + const mine = generation.fetchAdd(1, .acq_rel) + 1; + if (!markStart(fs, current)) { + std.log.warn("android: crash guard: could not write {s}", .{start_file}); + return; + } + spawner.spawn(mine) catch |err| { + std.log.warn("android: crash guard: could not start the stable timer ({s}); keeping {s}, so the next launch uses gles", .{ @errorName(err), start_file }); + }; +} + /// The stable timer: sleep `stable_after_ns`, then delete the start mark, /// unless a newer `beginVulkanStart` (another activity launch in the same /// process) has taken over (`generation` moved past `mine`); its own timer -/// will clear the mark. `clock.sleep(ns) !void` — a fake returns an error to +/// will clear the mark. +/// TODO(#172 D11): this runs from `renderer.resolve`, before `bgfx.init`, so +/// a Vulkan init hanging past 10 s clears the mark (Codex review of +/// labelle-android#33, labelle-bgfx#172 comment 5904099277). Needs a +/// lifecycle signal; waiting on an owner decision. `clock.sleep(ns) !void` — a fake returns an error to /// model the process dying mid-sleep. pub fn settle(fs: anytype, clock: anytype, generation: *const std.atomic.Value(u32), mine: u32) void { clock.sleep(stable_after_ns) catch return; @@ -242,22 +279,18 @@ pub fn beginVulkanStart(activity: ?*const anyopaque, setting: []const u8) void { if (comptime !is_android) return; const a = activity orelse return; const fs = libcFs(a) orelse return; - const mine = start_generation.fetchAdd(1, .acq_rel) + 1; - if (!markStart(&fs, .{ .version_code = versionCode(a), .setting = setting })) { - std.log.warn("android: crash guard: could not write {s}", .{start_file}); - return; - } - const t = std.Thread.spawn(.{}, stableThread, .{ fs, mine }) catch |err| { - // No timer: the mark stays and the NEXT launch uses GLES. Safer to - // drop the mark now and run unguarded than to disable Vulkan for a - // start that may well succeed. - std.log.warn("android: crash guard: could not start the stable timer ({s}); guard off for this launch", .{@errorName(err)}); - fs.delete(start_file); - return; - }; - t.detach(); + beginStart(&fs, .{ .version_code = versionCode(a), .setting = setting }, &start_generation, ThreadSpawner{ .fs = fs }); } +/// Spawns the detached stable-timer thread with its own copy of the fs. +const ThreadSpawner = struct { + fs: LibcFs, + pub fn spawn(self: ThreadSpawner, mine: u32) std.Thread.SpawnError!void { + const t = try std.Thread.spawn(.{}, stableThread, .{ self.fs, mine }); + t.detach(); + } +}; + fn stableThread(fs: LibcFs, mine: u32) void { settle(&fs, LibcClock{}, &start_generation, mine); } @@ -371,12 +404,55 @@ test "a crash inside 10 s disables Vulkan at the next launch" { try testing.expectEqualStrings("1 vulkan", fs.disabled.?); } -test "a failed disabled write still keeps THIS launch off Vulkan" { +test "a failed disabled write keeps the start mark (fail closed): still guarded next launch" { var fs: FakeFs = .{}; fs.set(start_file, "1 vulkan"); fs.fail_writes = true; try testing.expect(checkAtLaunch(&fs, v1_vulkan)); - try testing.expect(fs.start == null and fs.disabled == null); + try testing.expect(fs.disabled == null); + try testing.expectEqualStrings("1 vulkan", fs.start.?); // kept + // Next launch: still guarded by the kept start mark... + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.start.?); + // ...and once writes work again, it converts to the disabled mark. + fs.fail_writes = false; + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.start == null); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); +} + +test "a stable timer that cannot start keeps the start mark (fail closed)" { + const FailingSpawner = struct { + calls: *usize, + pub fn spawn(self: @This(), _: u32) error{SystemResources}!void { + self.calls.* += 1; + return error.SystemResources; + } + }; + var fs: FakeFs = .{}; + var gen: std.atomic.Value(u32) = .init(0); + var calls: usize = 0; + beginStart(&fs, v1_vulkan, &gen, FailingSpawner{ .calls = &calls }); + try testing.expectEqual(@as(usize, 1), calls); + try testing.expectEqualStrings("1 vulkan", fs.start.?); // kept + // The next launch treats it as an incomplete start: gles. + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); +} + +test "beginStart: mark written, timer spawned with the new generation" { + const RecordingSpawner = struct { + got: *?u32, + pub fn spawn(self: @This(), mine: u32) error{}!void { + self.got.* = mine; + } + }; + var fs: FakeFs = .{}; + var gen: std.atomic.Value(u32) = .init(4); + var got: ?u32 = null; + beginStart(&fs, v1_vulkan, &gen, RecordingSpawner{ .got = &got }); + try testing.expectEqual(@as(?u32, 5), got); + try testing.expectEqualStrings("1 vulkan", fs.start.?); } test "surviving 10 s clears the mark; the next launch tries Vulkan again" { @@ -481,6 +557,13 @@ test "an intent override beats the guard (renderer.decide asks the intent first) try testing.expectEqual(renderer.Source.intent, d.source); try testing.expectEqual(@as(usize, 0), q.guard_calls); + // Debuggable WITHOUT the extra: the guard stays active (option c). + var n: Q = .{ .fs = &fs, .extra = null, .is_debuggable = true }; + const g = renderer.decide(&n); + try testing.expectEqual(renderer.Renderer.gles, g.renderer); + try testing.expectEqual(renderer.Source.crash_guard, g.source); + try testing.expectEqual(@as(usize, 1), n.guard_calls); + // Not debuggable: the extra is ignored and the guard wins. var r: Q = .{ .fs = &fs, .extra = "vulkan", .is_debuggable = false }; const e = renderer.decide(&r); From 94c65ca13b158a352f9f941104eeedbf3b2ca445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 02:09:15 -0300 Subject: [PATCH 4/8] feat(crash_guard): stable = 120 presented frames AND 10 s (#28, owner option 1) - The stable thread polls every 500 ms and clears .labelle_vulkan_start only once >= 120 frames are presented AND >= 10 s have passed since beginVulkanStart; a hung Vulkan init never clears it. Generation guard kept (checked every poll). - Frames come from labelle-bgfx#182's labelle_bgfx_frames_presented(), looked up at runtime with dlsym(RTLD_DEFAULT) in renderer_query.c (links libdl; no build dependency on labelle-bgfx). Missing symbol -> time-only rule, logged once per process. - Pure settle(fs, clock, frames, ...) host-tested: frames-but-<10 s, 10 s-but-<120 frames (hung init + stall), both (either order), missing symbol (noted once), superseded mid-poll. - Drop the TODO(#172 D11) notes; docs describe the new rule. - F2: drop renderer.decide's duplicate "not debuggable" log; intent_env already logs it. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- build.zig | 3 + src/crash_guard.zig | 246 ++++++++++++++++++++++++++++++++------- src/jni/renderer_query.c | 27 ++++- src/renderer.zig | 8 +- 4 files changed, 239 insertions(+), 45 deletions(-) diff --git a/build.zig b/build.zig index 32879f2..d5f5bb5 100644 --- a/build.zig +++ b/build.zig @@ -142,6 +142,9 @@ pub fn build(b: *std.Build) void { }); mod.linkSystemLibrary("android", .{}); mod.linkSystemLibrary("log", .{}); + // `dlsym` (crash_guard's runtime lookup of labelle-bgfx's frame + // counter, labelle-android#28). + mod.linkSystemLibrary("dl", .{}); // `aaudio.zig` (the AAudio output device, phase 1c) is pure `extern fn` // against libaaudio (API 26+); the link propagates to any consumer // module that imports `labelle_android`. diff --git a/src/crash_guard.zig b/src/crash_guard.zig index 2aae5e5..43cfecc 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -14,14 +14,18 @@ //! never reached "stable". //! //! Rules: -//! * Stable: a detached thread deletes `.labelle_vulkan_start` once the -//! process has lived `stable_after_ns` (10 s) after `beginVulkanStart`. -//! No labelle-bgfx signal (the generic-only rule). -//! TODO(#172 D11): the timer starts at `renderer.resolve`, BEFORE -//! `bgfx.init`, so a Vulkan init that hangs for more than 10 s clears the -//! mark and is not caught (Codex review of labelle-android#33, -//! labelle-bgfx#172 comment 5904099277). The fix needs a lifecycle -//! signal; waiting on an owner decision. +//! * Stable: a detached thread polls (every `poll_interval_ns`, 500 ms) +//! and deletes `.labelle_vulkan_start` once BOTH hold: at least +//! `stable_min_frames` (120) frames presented, and at least +//! `stable_after_ns` (10 s) since `beginVulkanStart`. The frame count is +//! labelle-bgfx's generic `labelle_bgfx_frames_presented()` export +//! (labelle-toolkit/labelle-bgfx#182; reset to 0 on every successful +//! `bgfx.init`, +1 per `bgfx.frame()`), looked up at RUNTIME with +//! `dlsym(RTLD_DEFAULT, ...)`: no build dependency on labelle-bgfx. A +//! Vulkan init that hangs never presents a frame, so the mark is never +//! cleared and the next launch uses GLES. +//! If the symbol is missing (another backend, or an older labelle-bgfx) +//! the rule falls back to time only (10 s), logged once per process. //! * Disabled: while `.labelle_vulkan_disabled` exists and its stamp //! matches the current `versionCode` and setting, `vulkanDisabled` //! returns true and `renderer.decide` picks `gles` (source @@ -37,9 +41,9 @@ //! timer cannot be started, the start mark is KEPT, so the next launch //! is still guarded (it finds the mark again and goes to GLES). //! -//! By design, anything that ends the process inside the stable window counts -//! as an incomplete start, including a developer's early force-stop (e.g. -//! `am start -S` within 10 s of the previous launch): the next launch runs +//! By design, anything that ends the process before the start is stable +//! counts as an incomplete start, including a developer's early force-stop +//! (e.g. `am start -S` within 10 s of the previous launch): the next launch runs //! on GLES. To bypass the guard while iterating, launch a debuggable build //! with `--es LABELLE_BGFX_RENDERER vulkan` (the intent override). //! @@ -54,6 +58,11 @@ pub const disabled_file = ".labelle_vulkan_disabled"; /// How long the process must live after `beginVulkanStart` before the start /// counts as stable. pub const stable_after_ns: u64 = 10 * std.time.ns_per_s; +/// How many frames must have been presented (labelle-bgfx#182's counter) +/// before the start counts as stable. +pub const stable_min_frames: u64 = 120; +/// How often the stable thread re-checks. +pub const poll_interval_ns: u64 = 500 * std.time.ns_per_ms; /// What both marker files contain: ` `. pub const Stamp = struct { @@ -156,19 +165,36 @@ pub fn beginStart(fs: anytype, current: Stamp, generation: *std.atomic.Value(u32 }; } -/// The stable timer: sleep `stable_after_ns`, then delete the start mark, -/// unless a newer `beginVulkanStart` (another activity launch in the same -/// process) has taken over (`generation` moved past `mine`); its own timer -/// will clear the mark. -/// TODO(#172 D11): this runs from `renderer.resolve`, before `bgfx.init`, so -/// a Vulkan init hanging past 10 s clears the mark (Codex review of -/// labelle-android#33, labelle-bgfx#172 comment 5904099277). Needs a -/// lifecycle signal; waiting on an owner decision. `clock.sleep(ns) !void` — a fake returns an error to -/// model the process dying mid-sleep. -pub fn settle(fs: anytype, clock: anytype, generation: *const std.atomic.Value(u32), mine: u32) void { - clock.sleep(stable_after_ns) catch return; - if (generation.load(.acquire) != mine) return; - fs.delete(start_file); +/// The stable timer. Polls every `poll_interval_ns` and deletes the start +/// mark once `stable_after_ns` has passed since it started AND +/// `src.framesPresented()` is at least `stable_min_frames`. Stops without +/// deleting when a newer `beginVulkanStart` (another activity launch in the +/// same process) has taken over (`generation` moved past `mine`): its own +/// timer owns the mark now. +/// +/// Injected: +/// * `clock.now() u64` (monotonic ns) and `clock.sleep(ns) !void`; a fake +/// returns an error to model the process dying mid-sleep. +/// * `src.framesPresented() ?u64`: null = the labelle-bgfx symbol is +/// missing → time-only rule; then `src.noteTimeOnly()` is called once +/// (the glue logs once per process). +pub fn settle(fs: anytype, clock: anytype, src: anytype, generation: *const std.atomic.Value(u32), mine: u32) void { + const t0 = clock.now(); + var noted = false; + while (generation.load(.acquire) == mine) { + const frames = src.framesPresented(); + if (frames == null and !noted) { + noted = true; + src.noteTimeOnly(); + } + const elapsed = clock.now() -% t0; + const enough_frames = if (frames) |f| f >= stable_min_frames else true; + if (elapsed >= stable_after_ns and enough_frames) { + fs.delete(start_file); + return; + } + clock.sleep(poll_interval_ns) catch return; + } } // ── Android glue ──────────────────────────────────────────────────────── @@ -176,6 +202,8 @@ pub fn settle(fs: anytype, clock: anytype, generation: *const std.atomic.Value(u // `jni/renderer_query.c`. extern "c" fn labelle_android_internal_data_path(activity: ?*const anyopaque) ?[*:0]const u8; extern "c" fn labelle_android_version_code(activity: ?*const anyopaque, out: *c_longlong) c_int; +extern "c" fn labelle_android_frames_presented(out: *u64) c_int; +extern "c" fn labelle_android_monotonic_ns() u64; // libc. const FILE = opaque {}; extern "c" fn fopen(name: [*:0]const u8, mode: [*:0]const u8) ?*FILE; @@ -183,7 +211,7 @@ extern "c" fn fread(ptr: [*]u8, size: usize, n: usize, f: *FILE) usize; extern "c" fn fwrite(ptr: [*]const u8, size: usize, n: usize, f: *FILE) usize; extern "c" fn fclose(f: *FILE) c_int; extern "c" fn unlink(name: [*:0]const u8) c_int; -extern "c" fn sleep(seconds: c_uint) c_uint; +extern "c" fn usleep(usec: c_uint) c_int; /// Marker files under a copied `internalDataPath` (a value type, so the /// detached stable thread owns its own copy). @@ -223,13 +251,35 @@ const LibcFs = struct { }; const LibcClock = struct { + pub fn now(_: LibcClock) u64 { + return labelle_android_monotonic_ns(); + } pub fn sleep(_: LibcClock, ns: u64) error{}!void { - var left: c_uint = @intCast(std.math.divCeil(u64, ns, std.time.ns_per_s) catch unreachable); - // `sleep` returns the unslept seconds when a signal interrupts it. - while (left > 0) left = sleep_c(left); + // A signal may cut it short; the poll loop re-checks either way. + _ = usleep(@intCast(@min(ns / std.time.ns_per_us, 1_000_000))); } }; -const sleep_c = sleep; + +/// Set once the time-only fallback has been logged (once per process). +var time_only_logged: std.atomic.Value(bool) = .init(false); + +/// labelle-bgfx#182's frame counter via `dlsym` (`jni/renderer_query.c`). +const DlsymFrames = struct { + pub fn framesPresented(_: DlsymFrames) ?u64 { + var n: u64 = 0; + return if (labelle_android_frames_presented(&n) != 0) n else null; + } + pub fn noteTimeOnly(_: DlsymFrames) void { + if (logOnce(&time_only_logged)) { + std.log.warn("crash guard: labelle_bgfx_frames_presented not found; using the time-only stable rule", .{}); + } + } +}; + +/// True for exactly one caller over the flag's lifetime. +fn logOnce(flag: *std.atomic.Value(bool)) bool { + return !flag.swap(true, .acq_rel); +} /// Bumped by every `beginVulkanStart`; each stable thread only clears the /// mark if it is still the latest. @@ -274,7 +324,8 @@ pub fn vulkanDisabled(activity: ?*const anyopaque, setting: []const u8) bool { } /// Called by `renderer.resolve` once Vulkan is chosen, before bgfx starts: -/// write `.labelle_vulkan_start` and start the detached 10 s stable timer. +/// write `.labelle_vulkan_start` and start the detached stable thread +/// (120 frames and 10 s, or 10 s alone without labelle-bgfx#182). pub fn beginVulkanStart(activity: ?*const anyopaque, setting: []const u8) void { if (comptime !is_android) return; const a = activity orelse return; @@ -292,7 +343,7 @@ const ThreadSpawner = struct { }; fn stableThread(fs: LibcFs, mine: u32) void { - settle(&fs, LibcClock{}, &start_generation, mine); + settle(&fs, LibcClock{}, DlsymFrames{}, &start_generation, mine); } // ── Tests ─────────────────────────────────────────────────────────────── @@ -343,6 +394,9 @@ const FakeClock = struct { dies_at_ns: ?u64 = null, slept_ns: u64 = 0, + pub fn now(self: *FakeClock) u64 { + return self.now_ns; + } pub fn sleep(self: *FakeClock, ns: u64) error{ProcessDied}!void { if (self.dies_at_ns) |d| if (self.now_ns + ns >= d) { self.now_ns = d; @@ -353,18 +407,55 @@ const FakeClock = struct { } }; +/// labelle-bgfx#182's counter as a function of the fake clock: 0 until +/// `init_done_ns` (null = init never finishes, i.e. hangs), then `fps` +/// frames per second. `missing` = the symbol is not exported. +const FakeFrames = struct { + clock: *FakeClock, + fps: u64 = 60, + init_done_ns: ?u64 = 0, + missing: bool = false, + notes: usize = 0, + calls: usize = 0, + /// Bump this generation on the Nth `framesPresented` call (a newer + /// `beginVulkanStart` landing mid-poll). + supersede: ?struct { gen: *std.atomic.Value(u32), at_call: usize } = null, + + pub fn framesPresented(self: *FakeFrames) ?u64 { + self.calls += 1; + if (self.supersede) |sup| if (self.calls == sup.at_call) { + _ = sup.gen.fetchAdd(1, .acq_rel); + }; + if (self.missing) return null; + const done = self.init_done_ns orelse return 0; + if (self.clock.now_ns < done) return 0; + return (self.clock.now_ns - done) * self.fps / std.time.ns_per_s; + } + pub fn noteTimeOnly(self: *FakeFrames) void { + self.notes += 1; + } +}; + const v1_vulkan: Stamp = .{ .version_code = 1, .setting = "vulkan" }; /// One Vulkan launch: check the guard; if clear, mark the start and run the -/// stable timer on `clock`. +/// stable thread on `clock` with a healthy 60 fps renderer. fn launchVulkan(fs: *FakeFs, clock: *FakeClock, gen: *std.atomic.Value(u32), stamp: Stamp) bool { if (checkAtLaunch(fs, stamp)) return false; const mine = gen.fetchAdd(1, .acq_rel) + 1; if (!markStart(fs, stamp)) return true; - settle(fs, clock, gen, mine); + var frames: FakeFrames = .{ .clock = clock }; + settle(fs, clock, &frames, gen, mine); return true; } +/// Mark a start and run only the stable thread; returns the frame fake. +fn runSettle(fs: *FakeFs, clock: *FakeClock, frames: *FakeFrames) void { + var gen: std.atomic.Value(u32) = .init(1); + _ = markStart(fs, v1_vulkan); + settle(fs, clock, frames, &gen, 1); +} + test "stamp: format and parse round-trip; junk is rejected" { var buf: [stamp_cap]u8 = undefined; const text = v1_vulkan.format(&buf).?; @@ -390,8 +481,8 @@ test "a crash inside 10 s disables Vulkan at the next launch" { var gen: std.atomic.Value(u32) = .init(0); var clock: FakeClock = .{ .dies_at_ns = stable_after_ns - 1 }; try testing.expect(launchVulkan(&fs, &clock, &gen, v1_vulkan)); - // The timer never finished, so the mark survived the "crash". - try testing.expectEqual(@as(u64, 0), clock.slept_ns); + // The thread never saw 10 s, so the mark survived the "crash". + try testing.expect(clock.slept_ns < stable_after_ns); try testing.expectEqualStrings("1 vulkan", fs.start.?); // Next launch: the stale start becomes the disabled mark. @@ -455,12 +546,12 @@ test "beginStart: mark written, timer spawned with the new generation" { try testing.expectEqualStrings("1 vulkan", fs.start.?); } -test "surviving 10 s clears the mark; the next launch tries Vulkan again" { +test "surviving 10 s (with frames) clears the mark; the next launch tries Vulkan again" { var fs: FakeFs = .{}; var gen: std.atomic.Value(u32) = .init(0); var clock: FakeClock = .{}; try testing.expect(launchVulkan(&fs, &clock, &gen, v1_vulkan)); - try testing.expectEqual(stable_after_ns, clock.slept_ns); + try testing.expectEqual(stable_after_ns, clock.now_ns); try testing.expect(fs.start == null and fs.disabled == null); try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); try testing.expect(fs.disabled == null); @@ -471,11 +562,84 @@ test "a superseded stable timer leaves the newer start mark alone" { var gen: std.atomic.Value(u32) = .init(0); const first = gen.fetchAdd(1, .acq_rel) + 1; try testing.expect(markStart(&fs, v1_vulkan)); - _ = gen.fetchAdd(1, .acq_rel); // a second beginVulkanStart in the same process var clock: FakeClock = .{}; - settle(&fs, &clock, &gen, first); - try testing.expectEqual(stable_after_ns, clock.slept_ns); // it did sleep... - try testing.expect(fs.start != null); // ...but did not delete + // A second beginVulkanStart lands on the 3rd poll, before 10 s. + var frames: FakeFrames = .{ .clock = &clock, .supersede = .{ .gen = &gen, .at_call = 3 } }; + settle(&fs, &clock, &frames, &gen, first); + try testing.expectEqual(@as(usize, 3), frames.calls); // it polled... + try testing.expect(clock.now_ns < stable_after_ns); // ...stopped early... + try testing.expect(fs.start != null); // ...and did not delete +} + +test "stable: 120 frames but under 10 s -> not cleared" { + var fs: FakeFs = .{}; + var clock: FakeClock = .{ .dies_at_ns = stable_after_ns - 1 }; + var frames: FakeFrames = .{ .clock = &clock, .fps = 1000 }; + runSettle(&fs, &clock, &frames); + try testing.expect(frames.framesPresented().? >= stable_min_frames); // frames were there + try testing.expect(fs.start != null); + try testing.expectEqual(@as(usize, 0), frames.notes); +} + +test "stable: 10 s but under 120 frames (a hung init) -> not cleared" { + var fs: FakeFs = .{}; + // Init never finishes; the player kills it after a minute. + var clock: FakeClock = .{ .dies_at_ns = 60 * std.time.ns_per_s }; + var frames: FakeFrames = .{ .clock = &clock, .init_done_ns = null }; + runSettle(&fs, &clock, &frames); + try testing.expect(clock.now_ns >= stable_after_ns); + try testing.expect(fs.start != null); + + // Init finished but only a few frames (a stall after the first frames). + var fs2: FakeFs = .{}; + var clock2: FakeClock = .{ .dies_at_ns = 60 * std.time.ns_per_s }; + var frames2: FakeFrames = .{ .clock = &clock2, .fps = 1, .init_done_ns = 0 }; + runSettle(&fs2, &clock2, &frames2); + try testing.expect(frames2.framesPresented().? < stable_min_frames); + try testing.expect(fs2.start != null); +} + +test "stable: both 120 frames and 10 s -> cleared, at whichever comes last" { + // Frames first (60 fps: 120 at 2 s), then time: cleared at 10 s. + var fs: FakeFs = .{}; + var clock: FakeClock = .{}; + var frames: FakeFrames = .{ .clock = &clock }; + runSettle(&fs, &clock, &frames); + try testing.expect(fs.start == null); + try testing.expectEqual(stable_after_ns, clock.now_ns); + + // Time first (slow init: frames start at 9 s, 20 fps → 120 at 15 s). + var fs2: FakeFs = .{}; + var clock2: FakeClock = .{}; + var frames2: FakeFrames = .{ .clock = &clock2, .fps = 20, .init_done_ns = 9 * std.time.ns_per_s }; + runSettle(&fs2, &clock2, &frames2); + try testing.expect(fs2.start == null); + try testing.expectEqual(15 * std.time.ns_per_s, clock2.now_ns); +} + +test "stable: missing symbol -> time-only rule, noted once" { + var fs: FakeFs = .{}; + var clock: FakeClock = .{}; + var frames: FakeFrames = .{ .clock = &clock, .missing = true }; + runSettle(&fs, &clock, &frames); + try testing.expect(fs.start == null); + try testing.expectEqual(stable_after_ns, clock.now_ns); + try testing.expect(frames.calls > 1); // polled many times... + try testing.expectEqual(@as(usize, 1), frames.notes); // ...noted once + + // Before 10 s it is still not cleared. + var fs2: FakeFs = .{}; + var clock2: FakeClock = .{ .dies_at_ns = stable_after_ns - 1 }; + var frames2: FakeFrames = .{ .clock = &clock2, .missing = true }; + runSettle(&fs2, &clock2, &frames2); + try testing.expect(fs2.start != null); +} + +test "logOnce: the time-only log fires for exactly one caller per process" { + var flag: std.atomic.Value(bool) = .init(false); + try testing.expect(logOnce(&flag)); + try testing.expect(!logOnce(&flag)); + try testing.expect(!logOnce(&flag)); } test "a version bump resets both files and Vulkan is tried again" { diff --git a/src/jni/renderer_query.c b/src/jni/renderer_query.c index b728281..2b5e33a 100644 --- a/src/jni/renderer_query.c +++ b/src/jni/renderer_query.c @@ -12,7 +12,8 @@ // getPackageManager().hasSystemFeature("android.hardware.vulkan.version", // 0x401000) // Vulkan 1.1 // * for the crash guard (labelle-android#28): the package's version code -// and `ANativeActivity.internalDataPath` +// and `ANativeActivity.internalDataPath`, plus (not JNI) the runtime +// `dlsym` of labelle-bgfx's frame counter and a monotonic clock // // In C for the same reason as intent_extras.c: already declares the // JNI vtables. Off Android this is an empty TU. @@ -20,8 +21,11 @@ #include #include +#include #include #include +#include +#include // android.content.pm.PackageManager.GET_META_DATA (API 1). #define LABELLE_GET_META_DATA 0x00000080 @@ -235,4 +239,25 @@ int labelle_android_version_code(const void *activity_ptr, long long *out) { return result; } +// The crash guard's stable rule (labelle-android#28) counts presented frames +// through labelle-bgfx's generic export (labelle-toolkit/labelle-bgfx#182): +// uint64_t labelle_bgfx_frames_presented(void); +// looked up at RUNTIME, so labelle-android has no build dependency on +// labelle-bgfx and still links for other backends. Writes the count to +// `*out` and returns 1, or returns 0 when the symbol is not exported. +int labelle_android_frames_presented(uint64_t *out) { + typedef uint64_t (*frames_fn)(void); + frames_fn fn = (frames_fn)dlsym(RTLD_DEFAULT, "labelle_bgfx_frames_presented"); + if (fn == NULL || out == NULL) return 0; + *out = fn(); + return 1; +} + +// CLOCK_MONOTONIC in nanoseconds (0 if the clock is unavailable). +uint64_t labelle_android_monotonic_ns(void) { + struct timespec ts; + if (clock_gettime(CLOCK_MONOTONIC, &ts) != 0) return 0; + return (uint64_t)ts.tv_sec * 1000000000ull + (uint64_t)ts.tv_nsec; +} + #endif /* __ANDROID__ */ diff --git a/src/renderer.zig b/src/renderer.zig index e93d4f9..e651bea 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -21,8 +21,8 @@ //! `android.hardware.vulkan.version` >= 1.1 (0x401000), else `gles`. //! //! Then, when the result is `vulkan`, `crash_guard.beginVulkanStart` (the -//! start mark + the 10 s stable timer); `setenv("LABELLE_BGFX_RENDERER", -//! "vulkan"|"gles", 1)`; and one log line: +//! start mark + the stable thread: 120 frames and 10 s); +//! `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles", 1)`; and one log line: //! `renderer: (source: intent|crash-guard|setting|auto)`, with //! `; previous Vulkan start did not complete` after `crash-guard`. //! @@ -129,7 +129,9 @@ pub fn decide(q: anytype) Decision { // An empty extra counts as absent, as it does in `intent_env`. if (v.len > 0) { if (!q.debuggable()) { - std.log.info("android: ignoring intent extra {s}: the apk is not debuggable", .{env_name}); + // Not logged here: `launch_intent.apply` (intent_env's + // debuggable-only gate) already logs "ignoring intent extra + // LABELLE_BGFX_RENDERER: the apk is not debuggable" once. } else if (parseRenderer(v)) |r| { return .{ .renderer = r, .source = .intent }; } else { From fdea9694cc1cc5b5bd3746078b74baf08ff628c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 02:26:59 -0300 Subject: [PATCH 5/8] fix(crash_guard): serialise start-mark bump+write vs check+unlink (#33 review) Codex (labelle-bgfx#172 comment 5904658178): settle checked the generation at loop entry, then read frames/clock, then unlinked, so a newer beginStart landing in between had its fresh mark deleted by the old thread. StartState { lock, generation } (injected; the process has one): - beginStart: { generation += 1; write the start mark } under the lock - settle's final step: { generation == mine?; unlink } under the same lock - frames/clock reads stay outside; the loop-entry peek is advisory. Zig 0.16 has no std.Thread.Mutex, so it is std.atomic.Mutex spun on tryLock (tiny critical sections, near-zero contention). Deterministic test: a real beginStart runs inside poll 21 (t = 10 s, 600 frames: the poll that decides "clear"); the new mark survives. Verified it FAILS with the locked re-check removed. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- src/crash_guard.zig | 189 +++++++++++++++++++++++++++++++------------- 1 file changed, 136 insertions(+), 53 deletions(-) diff --git a/src/crash_guard.zig b/src/crash_guard.zig index 43cfecc..e9c35fd 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -150,13 +150,48 @@ pub fn markStart(fs: anytype, current: Stamp) bool { return fs.write(start_file, text); } -/// `beginVulkanStart`'s pure half: bump the generation, write the start mark, -/// start the stable timer via `spawner.spawn(mine: u32) !void`. Fails -/// CLOSED: if the timer cannot start, the mark is KEPT, so the next launch -/// runs on GLES rather than an unguarded Vulkan start being forgotten. -pub fn beginStart(fs: anytype, current: Stamp, generation: *std.atomic.Value(u32), spawner: anytype) void { - const mine = generation.fetchAdd(1, .acq_rel) + 1; - if (!markStart(fs, current)) { +/// The start mark's ownership, shared by `beginStart` and every stable +/// thread of the process. `lock` serialises the two steps that must not +/// interleave: +/// * `beginStart`: { generation += 1; write the start mark } +/// * `settle`'s final step: { generation still == mine?; unlink the mark } +/// so an old stable thread can never unlink a mark a newer launch just +/// wrote (Codex review of labelle-android#33, labelle-bgfx#172 comment +/// 5904658178). The frame counter and the clock are read OUTSIDE the lock. +/// +/// `std.atomic.Mutex` spun on `tryLock` (Zig 0.16 has no `std.Thread.Mutex`, +/// and `std.Io.Mutex` needs an `Io`): both critical sections are a few +/// syscalls on a tiny file and contention needs two activity launches in +/// the same instant, so a spin is fine. +pub const StartState = struct { + lock: std.atomic.Mutex = .unlocked, + /// Written only under `lock`; read without it (atomically) only for the + /// stable loop's early exit, which is advisory. + generation: u32 = 0, + + fn acquire(self: *StartState) void { + while (!self.lock.tryLock()) std.atomic.spinLoopHint(); + } + fn release(self: *StartState) void { + self.lock.unlock(); + } + fn peek(self: *const StartState) u32 { + return @atomicLoad(u32, &self.generation, .acquire); + } +}; + +/// `beginVulkanStart`'s pure half: under `state.lock`, bump the generation +/// and write the start mark; then start the stable thread via +/// `spawner.spawn(mine: u32) !void`. Fails CLOSED: if the thread cannot +/// start, the mark is KEPT, so the next launch runs on GLES rather than an +/// unguarded Vulkan start being forgotten. +pub fn beginStart(fs: anytype, current: Stamp, state: *StartState, spawner: anytype) void { + state.acquire(); + const mine = state.generation +% 1; + @atomicStore(u32, &state.generation, mine, .release); + const written = markStart(fs, current); + state.release(); + if (!written) { std.log.warn("android: crash guard: could not write {s}", .{start_file}); return; } @@ -165,12 +200,13 @@ pub fn beginStart(fs: anytype, current: Stamp, generation: *std.atomic.Value(u32 }; } -/// The stable timer. Polls every `poll_interval_ns` and deletes the start +/// The stable thread. Polls every `poll_interval_ns` and deletes the start /// mark once `stable_after_ns` has passed since it started AND -/// `src.framesPresented()` is at least `stable_min_frames`. Stops without -/// deleting when a newer `beginVulkanStart` (another activity launch in the -/// same process) has taken over (`generation` moved past `mine`): its own -/// timer owns the mark now. +/// `src.framesPresented()` is at least `stable_min_frames`. A newer +/// `beginStart` (another activity launch in the same process) takes the mark +/// over: this thread then stops without deleting. The final check-and-unlink +/// runs under `state.lock`, so it cannot interleave with a newer +/// `beginStart`'s bump-and-write. /// /// Injected: /// * `clock.now() u64` (monotonic ns) and `clock.sleep(ns) !void`; a fake @@ -178,10 +214,11 @@ pub fn beginStart(fs: anytype, current: Stamp, generation: *std.atomic.Value(u32 /// * `src.framesPresented() ?u64`: null = the labelle-bgfx symbol is /// missing → time-only rule; then `src.noteTimeOnly()` is called once /// (the glue logs once per process). -pub fn settle(fs: anytype, clock: anytype, src: anytype, generation: *const std.atomic.Value(u32), mine: u32) void { +pub fn settle(fs: anytype, clock: anytype, src: anytype, state: *StartState, mine: u32) void { const t0 = clock.now(); var noted = false; - while (generation.load(.acquire) == mine) { + // Advisory early exit; the authoritative check is under the lock below. + while (state.peek() == mine) { const frames = src.framesPresented(); if (frames == null and !noted) { noted = true; @@ -190,7 +227,9 @@ pub fn settle(fs: anytype, clock: anytype, src: anytype, generation: *const std. const elapsed = clock.now() -% t0; const enough_frames = if (frames) |f| f >= stable_min_frames else true; if (elapsed >= stable_after_ns and enough_frames) { - fs.delete(start_file); + state.acquire(); + defer state.release(); + if (state.generation == mine) fs.delete(start_file); return; } clock.sleep(poll_interval_ns) catch return; @@ -281,9 +320,8 @@ fn logOnce(flag: *std.atomic.Value(bool)) bool { return !flag.swap(true, .acq_rel); } -/// Bumped by every `beginVulkanStart`; each stable thread only clears the -/// mark if it is still the latest. -var start_generation: std.atomic.Value(u32) = .init(0); +/// The process's start-mark ownership (see `StartState`). +var start_state: StartState = .{}; /// Process-wide `versionCode` cache: it cannot change for the life of the /// process (an update kills it). -1 = not asked yet. @@ -330,7 +368,7 @@ pub fn beginVulkanStart(activity: ?*const anyopaque, setting: []const u8) void { if (comptime !is_android) return; const a = activity orelse return; const fs = libcFs(a) orelse return; - beginStart(&fs, .{ .version_code = versionCode(a), .setting = setting }, &start_generation, ThreadSpawner{ .fs = fs }); + beginStart(&fs, .{ .version_code = versionCode(a), .setting = setting }, &start_state, ThreadSpawner{ .fs = fs }); } /// Spawns the detached stable-timer thread with its own copy of the fs. @@ -343,7 +381,7 @@ const ThreadSpawner = struct { }; fn stableThread(fs: LibcFs, mine: u32) void { - settle(&fs, LibcClock{}, DlsymFrames{}, &start_generation, mine); + settle(&fs, LibcClock{}, DlsymFrames{}, &start_state, mine); } // ── Tests ─────────────────────────────────────────────────────────────── @@ -417,14 +455,15 @@ const FakeFrames = struct { missing: bool = false, notes: usize = 0, calls: usize = 0, - /// Bump this generation on the Nth `framesPresented` call (a newer - /// `beginVulkanStart` landing mid-poll). - supersede: ?struct { gen: *std.atomic.Value(u32), at_call: usize } = null, + /// Run a real `beginStart` (a newer launch in the same process: new + /// generation + new mark) inside the Nth `framesPresented` call, i.e. + /// after `settle`'s loop-entry check and before its stable decision. + restart: ?struct { at_call: usize, fs: *FakeFs, state: *StartState, stamp: Stamp } = null, pub fn framesPresented(self: *FakeFrames) ?u64 { self.calls += 1; - if (self.supersede) |sup| if (self.calls == sup.at_call) { - _ = sup.gen.fetchAdd(1, .acq_rel); + if (self.restart) |r| if (self.calls == r.at_call) { + beginStart(r.fs, r.stamp, r.state, NoSpawner{}); }; if (self.missing) return null; const done = self.init_done_ns orelse return 0; @@ -436,24 +475,36 @@ const FakeFrames = struct { } }; +const NoSpawner = struct { + pub fn spawn(_: NoSpawner, _: u32) error{}!void {} +}; + const v1_vulkan: Stamp = .{ .version_code = 1, .setting = "vulkan" }; +const v2_vulkan: Stamp = .{ .version_code = 2, .setting = "vulkan" }; /// One Vulkan launch: check the guard; if clear, mark the start and run the /// stable thread on `clock` with a healthy 60 fps renderer. -fn launchVulkan(fs: *FakeFs, clock: *FakeClock, gen: *std.atomic.Value(u32), stamp: Stamp) bool { +fn launchVulkan(fs: *FakeFs, clock: *FakeClock, state: *StartState, stamp: Stamp) bool { if (checkAtLaunch(fs, stamp)) return false; - const mine = gen.fetchAdd(1, .acq_rel) + 1; - if (!markStart(fs, stamp)) return true; + var got: ?u32 = null; + beginStart(fs, stamp, state, RecordingSpawner{ .got = &got }); var frames: FakeFrames = .{ .clock = clock }; - settle(fs, clock, &frames, gen, mine); + if (got) |mine| settle(fs, clock, &frames, state, mine); return true; } +const RecordingSpawner = struct { + got: *?u32, + pub fn spawn(self: RecordingSpawner, mine: u32) error{}!void { + self.got.* = mine; + } +}; + /// Mark a start and run only the stable thread; returns the frame fake. fn runSettle(fs: *FakeFs, clock: *FakeClock, frames: *FakeFrames) void { - var gen: std.atomic.Value(u32) = .init(1); - _ = markStart(fs, v1_vulkan); - settle(fs, clock, frames, &gen, 1); + var state: StartState = .{}; + beginStart(fs, v1_vulkan, &state, NoSpawner{}); + settle(fs, clock, frames, &state, state.generation); } test "stamp: format and parse round-trip; junk is rejected" { @@ -478,9 +529,9 @@ test "first start: nothing on disk, Vulkan allowed, start mark written" { test "a crash inside 10 s disables Vulkan at the next launch" { var fs: FakeFs = .{}; - var gen: std.atomic.Value(u32) = .init(0); + var state: StartState = .{}; var clock: FakeClock = .{ .dies_at_ns = stable_after_ns - 1 }; - try testing.expect(launchVulkan(&fs, &clock, &gen, v1_vulkan)); + try testing.expect(launchVulkan(&fs, &clock, &state, v1_vulkan)); // The thread never saw 10 s, so the mark survived the "crash". try testing.expect(clock.slept_ns < stable_after_ns); try testing.expectEqualStrings("1 vulkan", fs.start.?); @@ -521,9 +572,9 @@ test "a stable timer that cannot start keeps the start mark (fail closed)" { } }; var fs: FakeFs = .{}; - var gen: std.atomic.Value(u32) = .init(0); + var state: StartState = .{}; var calls: usize = 0; - beginStart(&fs, v1_vulkan, &gen, FailingSpawner{ .calls = &calls }); + beginStart(&fs, v1_vulkan, &state, FailingSpawner{ .calls = &calls }); try testing.expectEqual(@as(usize, 1), calls); try testing.expectEqualStrings("1 vulkan", fs.start.?); // kept // The next launch treats it as an incomplete start: gles. @@ -532,25 +583,19 @@ test "a stable timer that cannot start keeps the start mark (fail closed)" { } test "beginStart: mark written, timer spawned with the new generation" { - const RecordingSpawner = struct { - got: *?u32, - pub fn spawn(self: @This(), mine: u32) error{}!void { - self.got.* = mine; - } - }; var fs: FakeFs = .{}; - var gen: std.atomic.Value(u32) = .init(4); + var state: StartState = .{ .generation = 4 }; var got: ?u32 = null; - beginStart(&fs, v1_vulkan, &gen, RecordingSpawner{ .got = &got }); + beginStart(&fs, v1_vulkan, &state, RecordingSpawner{ .got = &got }); try testing.expectEqual(@as(?u32, 5), got); try testing.expectEqualStrings("1 vulkan", fs.start.?); } test "surviving 10 s (with frames) clears the mark; the next launch tries Vulkan again" { var fs: FakeFs = .{}; - var gen: std.atomic.Value(u32) = .init(0); + var state: StartState = .{}; var clock: FakeClock = .{}; - try testing.expect(launchVulkan(&fs, &clock, &gen, v1_vulkan)); + try testing.expect(launchVulkan(&fs, &clock, &state, v1_vulkan)); try testing.expectEqual(stable_after_ns, clock.now_ns); try testing.expect(fs.start == null and fs.disabled == null); try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); @@ -559,16 +604,54 @@ test "surviving 10 s (with frames) clears the mark; the next launch tries Vulkan test "a superseded stable timer leaves the newer start mark alone" { var fs: FakeFs = .{}; - var gen: std.atomic.Value(u32) = .init(0); - const first = gen.fetchAdd(1, .acq_rel) + 1; - try testing.expect(markStart(&fs, v1_vulkan)); + var state: StartState = .{}; + beginStart(&fs, v1_vulkan, &state, NoSpawner{}); + const first = state.generation; var clock: FakeClock = .{}; - // A second beginVulkanStart lands on the 3rd poll, before 10 s. - var frames: FakeFrames = .{ .clock = &clock, .supersede = .{ .gen = &gen, .at_call = 3 } }; - settle(&fs, &clock, &frames, &gen, first); + // A second beginStart lands on the 3rd poll, well before 10 s. + var frames: FakeFrames = .{ .clock = &clock, .restart = .{ .at_call = 3, .fs = &fs, .state = &state, .stamp = v2_vulkan } }; + settle(&fs, &clock, &frames, &state, first); try testing.expectEqual(@as(usize, 3), frames.calls); // it polled... try testing.expect(clock.now_ns < stable_after_ns); // ...stopped early... - try testing.expect(fs.start != null); // ...and did not delete + try testing.expectEqualStrings("2 vulkan", fs.start.?); // ...and left the new mark +} + +test "race: a newer start landing at the very poll that would clear keeps its mark" { + // Codex review (labelle-bgfx#172 comment 5904658178). Poll 21 is t = 10 s + // (500 ms polls from 0) with 600 frames at 60 fps, so it is the poll + // whose stable decision says "clear". The newer beginStart runs inside + // that poll, AFTER the loop-entry generation check. Without the locked + // re-check the old thread unlinks the new launch's mark (this test then + // fails: fs.start == null). + var fs: FakeFs = .{}; + var state: StartState = .{}; + beginStart(&fs, v1_vulkan, &state, NoSpawner{}); + const first = state.generation; + var clock: FakeClock = .{}; + var frames: FakeFrames = .{ .clock = &clock, .restart = .{ .at_call = 21, .fs = &fs, .state = &state, .stamp = v2_vulkan } }; + settle(&fs, &clock, &frames, &state, first); + try testing.expectEqual(@as(usize, 21), frames.calls); + try testing.expectEqual(stable_after_ns, clock.now_ns); // it reached the stable branch + try testing.expectEqual(first + 1, state.generation); + try testing.expect(fs.start != null); // the new mark survived... + try testing.expectEqualStrings("2 vulkan", fs.start.?); // ...and it IS the new one + try testing.expect(state.lock.tryLock()); // settle released the lock + state.release(); +} + +test "the stable thread's own clear still works under the lock" { + var fs: FakeFs = .{}; + var clock: FakeClock = .{}; + var frames: FakeFrames = .{ .clock = &clock }; + var state: StartState = .{}; + beginStart(&fs, v1_vulkan, &state, NoSpawner{}); + try testing.expect(state.lock.tryLock()); // beginStart released it... + try testing.expect(!state.lock.tryLock()); // ...and it is exclusive + state.release(); + settle(&fs, &clock, &frames, &state, state.generation); + try testing.expect(fs.start == null); + try testing.expect(state.lock.tryLock()); + state.release(); } test "stable: 120 frames but under 10 s -> not cleared" { From 1aaccbbf751391376c9916c53761c80086453dde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 02:50:32 -0300 Subject: [PATCH 6/8] fix(crash_guard): an unreadable setting or versionCode never resets the marks (#33 review) Codex (labelle-bgfx#172 comment 5904915798 + PR bot): a JNI read failure of the labelle.renderer meta-data was folded into "absent" -> gles, and a failed versionCode read into 0; either looked like a change and deleted a matching disabled/start mark, so the next good read retried the Vulkan start that had crashed. - renderer.MetaRead { value, absent, too_long, read_error } from the JNI helper's (already distinct) codes: >=0 len, -1 absent, -3 too long, -2/other read error. decide still gets gles for all non-values; the guard gets the effective setting for every successful read (absent -> gles) and null for read_error. Read once per launch (cached). - versionCode(): ?i64; only a successful read is cached, failure logged once, never a synthetic 0. - crash_guard.Observed { version_code: ?i64, setting: ?[]const u8 } + checkObserved: reset only on a SUCCESSFULLY read different value; an unknown half keeps both marks untouched and guards this launch. beginVulkanStart writes no mark without a full stamp (logged). - Host regression tests (through renderer.decide): meta read error, then a good read of the original setting -> still crash-guard (disabled and pending-start cases); the same for versionCode; an observed change with the other half unknown still resets; absent key = gles = a real change vs a vulkan stamp -> reset. Verified they fail with unknown treated as a change. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- src/crash_guard.zig | 253 +++++++++++++++++++++++++++++++++------ src/jni/renderer_query.c | 7 +- src/renderer.zig | 111 +++++++++++++---- 3 files changed, 313 insertions(+), 58 deletions(-) diff --git a/src/crash_guard.zig b/src/crash_guard.zig index e9c35fd..c95dedd 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -94,6 +94,39 @@ pub const Stamp = struct { /// Room for a stamp: a 20-digit i64, a space and the longest setting. const stamp_cap = 64; +/// What this launch could OBSERVE of the current stamp. A null field is a +/// READ FAILURE (JNI error), not a value: it can never count as a change, +/// so it never resets the guard (Codex review of labelle-android#33, +/// labelle-bgfx#172 comment 5904915798). An ABSENT `labelle.renderer` key is +/// a successful read and arrives here as the default `gles`. +pub const Observed = struct { + version_code: ?i64, + setting: ?[]const u8, + + pub fn of(stamp: Stamp) Observed { + return .{ .version_code = stamp.version_code, .setting = stamp.setting }; + } + /// The full stamp, when both halves were read. + pub fn full(self: Observed) ?Stamp { + return .{ .version_code = self.version_code orelse return null, .setting = self.setting orelse return null }; + } +}; + +const Match = enum { same, changed, unknown }; + +/// `changed` only on a SUCCESSFULLY read, different value; a failed read of +/// either half otherwise gives `unknown`. +fn compare(recorded: Stamp, now: Observed) Match { + if (now.version_code) |v| if (v != recorded.version_code) return .changed; + if (now.setting) |x| if (!std.mem.eql(u8, x, recorded.setting)) return .changed; + return if (now.full() == null) .unknown else .same; +} + +/// The launch-time check with a fully read stamp (see `checkObserved`). +pub fn checkAtLaunch(fs: anytype, current: Stamp) bool { + return checkObserved(fs, Observed.of(current)); +} + /// The launch-time check. `fs` provides: /// * `read(name: []const u8, buf: []u8) ?[]const u8` (null = absent or /// unreadable) @@ -102,38 +135,60 @@ const stamp_cap = 64; /// /// Returns true when this launch must not use Vulkan. Side effects, in order: /// 1. `.labelle_vulkan_disabled` present: matching stamp → true (and a -/// leftover start mark is dropped); otherwise reset (both files go). +/// leftover start mark is dropped); an observed change (or a garbled +/// stamp) → reset (both files go), false. /// 2. `.labelle_vulkan_start` present (the previous Vulkan start did not /// complete): matching stamp → write `.labelle_vulkan_disabled`, drop the -/// start mark, true; otherwise reset. If the write fails, the start -/// mark is KEPT (fail closed): the next launch finds it again and is -/// still guarded. +/// start mark, true; an observed change (or garbled) → reset, false. If +/// the write fails, the start mark is KEPT (fail closed): the next +/// launch finds it again and is still guarded. /// 3. Neither → false. -pub fn checkAtLaunch(fs: anytype, current: Stamp) bool { +/// A mark whose stamp cannot be compared because the version or setting +/// could not be READ (`unknown`) is PRESERVED untouched and this launch is +/// treated as guarded (true): the next launch that reads both decides. +pub fn checkObserved(fs: anytype, current: Observed) bool { var buf: [stamp_cap]u8 = undefined; if (fs.read(disabled_file, &buf)) |bytes| { - if (Stamp.parse(bytes)) |s| if (s.eql(current)) { - fs.delete(start_file); - return true; + const s = Stamp.parse(bytes) orelse { + reset(fs); + return false; }; - reset(fs); - return false; + switch (compare(s, current)) { + .same => { + fs.delete(start_file); + return true; + }, + .unknown => return true, + .changed => { + reset(fs); + return false; + }, + } } if (fs.read(start_file, &buf)) |bytes| { - if (Stamp.parse(bytes)) |s| if (s.eql(current)) { - var out: [stamp_cap]u8 = undefined; - const written = if (current.format(&out)) |text| fs.write(disabled_file, text) else false; - if (written) { - fs.delete(start_file); - } else { - // Fail closed: keep the start mark so the NEXT launch is - // still guarded. This launch is off Vulkan either way. - std.log.warn("android: crash guard: could not write {s}; keeping {s}", .{ disabled_file, start_file }); - } - return true; + const s = Stamp.parse(bytes) orelse { + reset(fs); + return false; }; - reset(fs); - return false; + switch (compare(s, current)) { + .same => { + var out: [stamp_cap]u8 = undefined; + const written = if (s.format(&out)) |text| fs.write(disabled_file, text) else false; + if (written) { + fs.delete(start_file); + } else { + // Fail closed: keep the start mark so the NEXT launch is + // still guarded. This launch is off Vulkan either way. + std.log.warn("android: crash guard: could not write {s}; keeping {s}", .{ disabled_file, start_file }); + } + return true; + }, + .unknown => return true, + .changed => { + reset(fs); + return false; + }, + } } return false; } @@ -324,18 +379,26 @@ fn logOnce(flag: *std.atomic.Value(bool)) bool { var start_state: StartState = .{}; /// Process-wide `versionCode` cache: it cannot change for the life of the -/// process (an update kills it). -1 = not asked yet. +/// process (an update kills it). -1 = not read yet. Only a SUCCESSFUL read +/// is cached, so a transient JNI failure is retried on the next call. var version_cache: std.atomic.Value(i64) = .init(-1); +var version_failure_logged: std.atomic.Value(bool) = .init(false); -fn versionCode(activity: *const anyopaque) i64 { +/// The app's versionCode, or null when the JNI read failed (never a +/// synthetic value: a fake 0 would look like a version change and reset the +/// guard). +fn versionCode(activity: *const anyopaque) ?i64 { const cached = version_cache.load(.acquire); if (cached >= 0) return cached; var v: c_longlong = 0; - const got: i64 = if (labelle_android_version_code(activity, &v) == 0 and v >= 0) v else 0; - // First store wins; only the winner logs, so the failure is logged once. - if (version_cache.cmpxchgStrong(-1, got, .acq_rel, .acquire)) |theirs| return theirs; - if (got == 0) std.log.warn("android: crash guard: could not read the app's versionCode; using 0", .{}); - return got; + if (labelle_android_version_code(activity, &v) != 0 or v < 0) { + if (logOnce(&version_failure_logged)) { + std.log.warn("android: crash guard: could not read the app's versionCode; keeping the guard's marks as they are", .{}); + } + return null; + } + version_cache.store(v, .release); + return v; } fn libcFs(activity: *const anyopaque) ?LibcFs { @@ -353,22 +416,31 @@ fn libcFs(activity: *const anyopaque) ?LibcFs { /// version and `setting` are unchanged since), so this launch must use GLES. /// Also applies the reset rule. `activity` is the running `ANativeActivity*` /// (opaque); `setting` the effective provider setting (`gles`|`vulkan`| -/// `auto`). Off Android, or with no activity or data dir: false. -pub fn vulkanDisabled(activity: ?*const anyopaque, setting: []const u8) bool { +/// `auto`), or null when the meta-data could not be READ (then, like a +/// failed versionCode read, the marks are preserved: see `checkObserved`). +/// Off Android, or with no activity or data dir: false. +pub fn vulkanDisabled(activity: ?*const anyopaque, setting: ?[]const u8) bool { if (comptime !is_android) return false; const a = activity orelse return false; var fs = libcFs(a) orelse return false; - return checkAtLaunch(&fs, .{ .version_code = versionCode(a), .setting = setting }); + return checkObserved(&fs, .{ .version_code = versionCode(a), .setting = setting }); } /// Called by `renderer.resolve` once Vulkan is chosen, before bgfx starts: /// write `.labelle_vulkan_start` and start the detached stable thread /// (120 frames and 10 s, or 10 s alone without labelle-bgfx#182). -pub fn beginVulkanStart(activity: ?*const anyopaque, setting: []const u8) void { +/// `setting` null = the meta-data could not be read. A start mark needs the +/// full stamp, so with either half unreadable no mark is written (logged). +pub fn beginVulkanStart(activity: ?*const anyopaque, setting: ?[]const u8) void { if (comptime !is_android) return; const a = activity orelse return; const fs = libcFs(a) orelse return; - beginStart(&fs, .{ .version_code = versionCode(a), .setting = setting }, &start_state, ThreadSpawner{ .fs = fs }); + const obs: Observed = .{ .version_code = versionCode(a), .setting = setting }; + const stamp = obs.full() orelse { + std.log.warn("android: crash guard: versionCode or {s} unreadable; no start mark for this Vulkan start", .{"labelle.renderer"}); + return; + }; + beginStart(&fs, stamp, &start_state, ThreadSpawner{ .fs = fs }); } /// Spawns the detached stable-timer thread with its own copy of the fs. @@ -835,3 +907,114 @@ test "Android: the glue is analysed (compile-check only; nothing runs)" { _ = &stableThread; _ = &@import("renderer.zig").resolve; } + +/// One launch through `renderer.decide` with the guard wired as the glue +/// wires it: the meta-data read's outcome (`renderer.MetaRead`) and a +/// versionCode read that may fail (null). +const LaunchQuery = struct { + const renderer = @import("renderer.zig"); + fs: *FakeFs, + meta: renderer.MetaRead, + version: ?i64, + + pub fn intentExtra(_: *@This()) ?[]const u8 { + return null; + } + pub fn debuggable(_: *@This()) bool { + return false; + } + pub fn vulkanDisabled(self: *@This()) bool { + return checkObserved(self.fs, .{ .version_code = self.version, .setting = self.meta.guardSetting() }); + } + pub fn metaData(self: *@This()) ?[]const u8 { + return self.meta.metaData(); + } + pub fn hasVulkan(_: *@This()) bool { + return true; + } + + fn run(fs: *FakeFs, meta: renderer.MetaRead, version: ?i64) renderer.Decision { + var q: LaunchQuery = .{ .fs = fs, .meta = meta, .version = version }; + return renderer.decide(&q); + } +}; + +test "a failed meta-data read preserves the marks; the next good read is still guarded" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + fs.set(start_file, "1 vulkan"); // a leftover start mark too + + // Launch with a JNI read error: gles for this launch, marks untouched. + const d1 = LaunchQuery.run(&fs, .read_error, 1); + try testing.expectEqual(R.Renderer.gles, d1.renderer); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); + try testing.expectEqualStrings("1 vulkan", fs.start.?); + + // Next launch reads the ORIGINAL setting: still disabled by the guard. + const d2 = LaunchQuery.run(&fs, .{ .value = "vulkan" }, 1); + try testing.expectEqual(R.Renderer.gles, d2.renderer); + try testing.expectEqual(R.Source.crash_guard, d2.source); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); +} + +test "a failed meta-data read also keeps a pending start mark for the next good read" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + fs.set(start_file, "1 auto"); // crashed on an auto-chosen Vulkan + _ = LaunchQuery.run(&fs, .read_error, 1); + try testing.expectEqualStrings("1 auto", fs.start.?); + try testing.expect(fs.disabled == null); + const d = LaunchQuery.run(&fs, .{ .value = "auto" }, 1); + try testing.expectEqual(R.Source.crash_guard, d.source); + try testing.expectEqualStrings("1 auto", fs.disabled.?); +} + +test "a failed versionCode read preserves the marks; the next good read is still guarded" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + fs.set(disabled_file, "42 vulkan"); + + // versionCode unreadable: guarded this launch, marks untouched. + const d1 = LaunchQuery.run(&fs, .{ .value = "vulkan" }, null); + try testing.expectEqual(R.Renderer.gles, d1.renderer); + try testing.expectEqual(R.Source.crash_guard, d1.source); + try testing.expectEqualStrings("42 vulkan", fs.disabled.?); + + const d2 = LaunchQuery.run(&fs, .{ .value = "vulkan" }, 42); + try testing.expectEqual(R.Source.crash_guard, d2.source); + try testing.expectEqualStrings("42 vulkan", fs.disabled.?); + + // Same for a pending start mark. + var fs2: FakeFs = .{}; + fs2.set(start_file, "42 vulkan"); + try testing.expect(checkObserved(&fs2, .{ .version_code = null, .setting = "vulkan" })); + try testing.expectEqualStrings("42 vulkan", fs2.start.?); + try testing.expect(fs2.disabled == null); + const d3 = LaunchQuery.run(&fs2, .{ .value = "vulkan" }, 42); + try testing.expectEqual(R.Source.crash_guard, d3.source); + try testing.expectEqualStrings("42 vulkan", fs2.disabled.?); +} + +test "an observed change still resets even when the OTHER half is unreadable" { + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + // A successfully read new version, setting unreadable: a real change. + try testing.expect(!checkObserved(&fs, .{ .version_code = 2, .setting = null })); + try testing.expect(fs.disabled == null); + fs.set(disabled_file, "1 vulkan"); + // A successfully read different setting, version unreadable. + try testing.expect(!checkObserved(&fs, .{ .version_code = null, .setting = "gles" })); + try testing.expect(fs.disabled == null); +} + +test "an absent key is a successful read of the default gles: a real change vs a vulkan stamp" { + const R = LaunchQuery.renderer; + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + fs.set(start_file, "1 vulkan"); + const d = LaunchQuery.run(&fs, .absent, 1); + try testing.expectEqual(R.Renderer.gles, d.renderer); + try testing.expectEqual(R.Source.setting, d.source); // not the guard + try testing.expect(fs.disabled == null and fs.start == null); // reset +} diff --git a/src/jni/renderer_query.c b/src/jni/renderer_query.c index 2b5e33a..249dcf7 100644 --- a/src/jni/renderer_query.c +++ b/src/jni/renderer_query.c @@ -48,6 +48,10 @@ static jobject package_manager(JNIEnv *env, jobject activity) { // Copy the `labelle.renderer` meta-data string into `buf` (NUL-terminated). // Returns its length (>= 0), -1 when the key is absent (or not a string), // -2 on any JNI failure, -3 when the value does not fit in `buf_cap`. +// -1 (absent) and -2 (failure) MUST stay distinct: `renderer.MetaRead` maps +// -1 to the default `gles` but -2 to "unknown", and the crash guard keeps its +// marks on unknown rather than reading a transient failure as a setting +// change (labelle-android#33 review). // Never leaves a Java exception pending and never leaks a local ref. // Callable from any thread (attached or not). int labelle_android_read_renderer_meta(const void *activity_ptr, char *buf, size_t buf_cap) { @@ -174,7 +178,8 @@ const char *labelle_android_internal_data_path(const void *activity_ptr) { // .getLongVersionCode() // API 28+ // .versionCode // older: NoSuchMethodError -> the int field // Writes it to `*out` and returns 0, or returns -1 on any JNI failure -// (`*out` untouched). Never leaves a Java exception pending and never leaks a +// (`*out` untouched; the Zig side treats it as "unknown", never as a +// version, so a failure cannot reset the crash guard). Never leaves a Java exception pending and never leaks a // local ref. Callable from any thread (attached or not). int labelle_android_version_code(const void *activity_ptr, long long *out) { const ANativeActivity *na = (const ANativeActivity *)activity_ptr; diff --git a/src/renderer.zig b/src/renderer.zig index e651bea..2072424 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -91,6 +91,51 @@ pub const Decision = struct { renderer: Renderer, source: Source }; /// The provider setting's values. pub const Setting = enum { gles, vulkan, auto }; +/// The three (four) outcomes of the `labelle.renderer` meta-data read. The +/// JNI helper's return code carries them across the C boundary: +/// `>= 0` = value length, `-1` = absent, `-3` = too long, `-2` (or any +/// other negative) = read error. +pub const MetaRead = union(enum) { + /// Read successfully (may still be an invalid value; `decide` warns). + value: []const u8, + /// Read successfully; the key is not there → the default, `gles`. + absent, + /// Read successfully; the value does not fit the buffer (so it is not a + /// valid setting) → `gles`, like any invalid value. + too_long, + /// The JNI walk failed: the setting is UNKNOWN. + read_error, + + pub fn fromCode(n: c_int, buf: []const u8) MetaRead { + if (n >= 0) return .{ .value = buf[0..@intCast(n)] }; + return switch (n) { + -1 => .absent, + -3 => .too_long, + else => .read_error, + }; + } + + /// What the setting rule sees: the value, or null (→ `gles`) otherwise. + /// For the RENDERER, a read error still means gles, as before. + pub fn metaData(self: MetaRead) ?[]const u8 { + return switch (self) { + .value => |v| v, + else => null, + }; + } + + /// The setting the crash guard compares/records: the effective setting + /// for every SUCCESSFUL read (absent/invalid/too long → `gles`), and null + /// for a read error, which the guard treats as unknown (marks kept). + pub fn guardSetting(self: MetaRead) ?[]const u8 { + return switch (self) { + .value => |v| settingLabel(v), + .absent, .too_long => "gles", + .read_error => null, + }; + } +}; + /// The effective setting the crash guard's stamp records: the parsed /// meta-data, and `gles` for missing or invalid (which is what they mean). pub fn settingLabel(meta: ?[]const u8) []const u8 { @@ -168,7 +213,7 @@ const JniQuery = struct { meta_buf: [64]u8 = undefined, /// The meta-data is asked for by the guard (for its stamp) and by the /// setting rule; the JNI walk (and its log line) happens once. - meta: ?(?[]const u8) = null, + meta: ?MetaRead = null, pub fn intentExtra(self: *JniQuery) ?[]const u8 { return self.extra; @@ -177,31 +222,23 @@ const JniQuery = struct { return debuggable_mod.isDebuggable(self.activity); } pub fn vulkanDisabled(self: *JniQuery) bool { - return crash_guard.vulkanDisabled(self.activity, settingLabel(self.metaData())); + return crash_guard.vulkanDisabled(self.activity, self.metaRead().guardSetting()); } pub fn metaData(self: *JniQuery) ?[]const u8 { - if (self.meta) |m| return m; - const m = self.readMeta(); - self.meta = m; - return m; + return self.metaRead().metaData(); } - fn readMeta(self: *JniQuery) ?[]const u8 { + fn metaRead(self: *JniQuery) MetaRead { + if (self.meta) |m| return m; const n = labelle_android_read_renderer_meta(self.activity, &self.meta_buf, self.meta_buf.len); - switch (n) { - -1 => { - std.log.info("android: no {s} meta-data; using gles", .{meta_data_name}); - return null; - }, - -3 => { - std.log.warn("android: {s} meta-data too long; using gles", .{meta_data_name}); - return null; - }, - else => if (n < 0) { - std.log.warn("android: could not read the {s} meta-data; using gles", .{meta_data_name}); - return null; - }, + const m = MetaRead.fromCode(n, &self.meta_buf); + switch (m) { + .value => {}, + .absent => std.log.info("android: no {s} meta-data; using gles", .{meta_data_name}), + .too_long => std.log.warn("android: {s} meta-data too long; using gles", .{meta_data_name}), + .read_error => std.log.warn("android: could not read the {s} meta-data; using gles (crash-guard marks kept)", .{meta_data_name}), } - return self.meta_buf[0..@intCast(n)]; + self.meta = m; + return m; } pub fn hasVulkan(self: *JniQuery) bool { return labelle_android_has_system_feature(self.activity, "android.hardware.vulkan.version", vulkan_1_1) != 0; @@ -218,7 +255,7 @@ pub fn resolve(activity: ?*const anyopaque, intent_extra: ?[:0]const u8) void { const a = activity orelse return; var q: JniQuery = .{ .activity = a, .extra = intent_extra }; const d = decide(&q); - if (d.renderer == .vulkan) crash_guard.beginVulkanStart(a, settingLabel(q.metaData())); + if (d.renderer == .vulkan) crash_guard.beginVulkanStart(a, q.metaRead().guardSetting()); if (setenv(env_name.ptr, d.renderer.envValue().ptr, 1) != 0) { std.log.warn("android: could not set {s}={s}", .{ env_name, d.renderer.envValue() }); } @@ -360,6 +397,36 @@ test "env values and log labels" { try testing.expectEqualStrings("", Source.setting.detail()); } +test "MetaRead: the JNI return code's outcomes, and what each side sees" { + const buf = "vulkanXXXX"; + const v = MetaRead.fromCode(6, buf); + try testing.expectEqualStrings("vulkan", v.value); + try testing.expectEqualStrings("vulkan", v.metaData().?); + try testing.expectEqualStrings("vulkan", v.guardSetting().?); + + // Absent: a successful read → default gles for both sides. + const a = MetaRead.fromCode(-1, buf); + try testing.expect(a == .absent); + try testing.expect(a.metaData() == null); + try testing.expectEqualStrings("gles", a.guardSetting().?); + + // Too long: an (invalid) value → gles. + const t = MetaRead.fromCode(-3, buf); + try testing.expect(t == .too_long); + try testing.expectEqualStrings("gles", t.guardSetting().?); + + // Read error: the renderer still gets gles, the guard gets UNKNOWN. + inline for (.{ -2, -7 }) |code| { + const e = MetaRead.fromCode(code, buf); + try testing.expect(e == .read_error); + try testing.expect(e.metaData() == null); + try testing.expect(e.guardSetting() == null); + } + + // An invalid value is still a successful read: effective gles. + try testing.expectEqualStrings("gles", MetaRead.fromCode(5, "metal").guardSetting().?); +} + test "settingLabel: the effective setting for the crash guard's stamp" { try testing.expectEqualStrings("vulkan", settingLabel("vulkan")); try testing.expectEqualStrings("auto", settingLabel("auto")); From 7c280351de4d8bc6bd6632ae5018d301e355924c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 02:56:26 -0300 Subject: [PATCH 7/8] fix(crash_guard): address the six open Codex-bot findings on #33 1. (P1) Same-process relaunch: StartState.owned (under the lock) marks a start mark THIS process wrote and still owns; checkObserved skips it, so a relaunched Activity supersedes it (new generation + mark) instead of disabling Vulkan. A dead process's mark (fresh state) is still a crash. 2. (P1) Per-start frame baseline: beginStart reads the counter and passes it to the stable thread; framesSince counts from it, and a value below it (bgfx re-init reset) drops the baseline to 0 for good. A hung new init can no longer be cleared by the previous Activity's frames. 3. (P2) Each marker is judged on its own stamp: a mismatching one is deleted alone; a matching start mark still counts as a crash. 4. (P2) Atomic marker writes (.tmp + rename, tmp removed on failure); a malformed marker is never a reset: fail closed, rewritten for the current stamp. 5. (P2) beginStart/beginVulkanStart return whether the start was recorded; renderer.afterStart runs the launch on gles (source crash-guard; "could not record the Vulkan start") when it was not, except for the intent override. 6. (P2) Markers live in getNoBackupFilesDir() (new JNI helper, same exception discipline); internalDataPath fallback, logged once. Host tests for 1-5 (each verified to fail with its fix reverted); the NDK compile-check covers the new JNI helper. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- src/crash_guard.zig | 600 ++++++++++++++++++++++++++++----------- src/jni/renderer_query.c | 56 +++- src/renderer.zig | 34 ++- 3 files changed, 520 insertions(+), 170 deletions(-) diff --git a/src/crash_guard.zig b/src/crash_guard.zig index c95dedd..a701f06 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -3,43 +3,58 @@ //! If the game crashes (or is killed) while starting on Vulkan, the next //! launch starts on GLES, so a bad Vulkan driver cannot brick the game. //! -//! Two marker files in the app's internal data dir -//! (`ANativeActivity.internalDataPath`), each containing -//! ` `: +//! Two marker files, each containing ` `, in the app's +//! `Context.getNoBackupFilesDir()` (`/no_backup`), which Android Auto +//! Backup and device-to-device transfer never copy: a crash on one device's +//! GPU must not disable Vulkan on another. If that JNI lookup fails, the +//! glue falls back to `ANativeActivity.internalDataPath` (logged once). //! //! * `.labelle_vulkan_start`: written by `beginVulkanStart`, just before a //! Vulkan start (the resolved renderer is `vulkan`). -//! * `.labelle_vulkan_disabled`: written when a launch finds -//! `.labelle_vulkan_start` still there, i.e. the previous Vulkan start -//! never reached "stable". +//! * `.labelle_vulkan_disabled`: written when a launch finds a +//! `.labelle_vulkan_start` left by a process that is gone, i.e. a Vulkan +//! start that never reached "stable". +//! +//! Both are written atomically (`.tmp` + `rename`), so a failed or +//! partial write never leaves a malformed marker behind. //! //! Rules: //! * Stable: a detached thread polls (every `poll_interval_ns`, 500 ms) //! and deletes `.labelle_vulkan_start` once BOTH hold: at least -//! `stable_min_frames` (120) frames presented, and at least -//! `stable_after_ns` (10 s) since `beginVulkanStart`. The frame count is -//! labelle-bgfx's generic `labelle_bgfx_frames_presented()` export -//! (labelle-toolkit/labelle-bgfx#182; reset to 0 on every successful -//! `bgfx.init`, +1 per `bgfx.frame()`), looked up at RUNTIME with -//! `dlsym(RTLD_DEFAULT, ...)`: no build dependency on labelle-bgfx. A -//! Vulkan init that hangs never presents a frame, so the mark is never -//! cleared and the next launch uses GLES. +//! `stable_min_frames` (120) frames presented SINCE THIS START, and at +//! least `stable_after_ns` (10 s) since `beginVulkanStart`. The frame +//! count is labelle-bgfx's generic `labelle_bgfx_frames_presented()` +//! export (labelle-toolkit/labelle-bgfx#182; reset to 0 on every +//! successful `bgfx.init`, +1 per `bgfx.frame()`), looked up at RUNTIME +//! with `dlsym(RTLD_DEFAULT, ...)`: no build dependency on labelle-bgfx. +//! `beginStart` records the counter as a per-start baseline, so frames a +//! previous Activity of the same process presented never count; a value +//! below the baseline means bgfx re-initialised, and counting restarts +//! from 0. A Vulkan init that hangs never presents a frame, so the mark +//! is never cleared and the next launch uses GLES. //! If the symbol is missing (another backend, or an older labelle-bgfx) //! the rule falls back to time only (10 s), logged once per process. +//! * Same process: a start mark THIS process wrote and still owns +//! (`StartState.owned`) is not a crash: an Activity relaunched in the +//! same process supersedes it (new generation, new mark) instead. //! * Disabled: while `.labelle_vulkan_disabled` exists and its stamp //! matches the current `versionCode` and setting, `vulkanDisabled` //! returns true and `renderer.decide` picks `gles` (source //! `crash-guard`). -//! * Reset: a stamp that does NOT match (a new app version or a changed -//! `renderer` setting) deletes both files, so Vulkan is tried again. An -//! unreadable stamp resets too. +//! * Reset: each marker whose stamp does NOT match (a successfully read +//! new app version or changed `renderer` setting) is deleted on its own, +//! so Vulkan is tried again; a marker that still matches is kept and +//! applied. A value that could not be READ never counts as a change. //! * Intent override: `renderer.decide` asks the debuggable-only intent //! extra BEFORE the guard, so it wins (see the test at the bottom). The //! guard itself stays ACTIVE in debuggable builds: only an explicit //! extra bypasses it. //! * Fails CLOSED: if the disabled mark cannot be written, or the stable //! timer cannot be started, the start mark is KEPT, so the next launch -//! is still guarded (it finds the mark again and goes to GLES). +//! is still guarded (it finds the mark again and goes to GLES). A +//! malformed marker is treated as a match for the current stamp. If the +//! start mark cannot be written at all, `renderer.resolve` runs this +//! launch on GLES (unless the intent override asked for Vulkan). //! //! By design, anything that ends the process before the start is stable //! counts as an incomplete start, including a developer's early force-stop @@ -47,9 +62,9 @@ //! on GLES. To bypass the guard while iterating, launch a debuggable build //! with `--es LABELLE_BGFX_RENDERER vulkan` (the intent override). //! -//! The PURE half (`checkAtLaunch`, `markStart`, `settle`) takes an injected -//! filesystem and clock and is host-tested; the Android glue below it uses -//! libc and the JNI helpers in `jni/renderer_query.c`. +//! The PURE half (`checkObserved`, `beginStart`, `settle`) takes an injected +//! filesystem, clock and frame source and is host-tested; the Android glue +//! below it uses libc and the JNI helpers in `jni/renderer_query.c`. const std = @import("std"); const is_android = @import("root.zig").is_android; @@ -122,107 +137,123 @@ fn compare(recorded: Stamp, now: Observed) Match { return if (now.full() == null) .unknown else .same; } -/// The launch-time check with a fully read stamp (see `checkObserved`). +/// The launch-time check of a NEW process (fresh `StartState`) with a fully +/// read stamp (see `checkObserved`). pub fn checkAtLaunch(fs: anytype, current: Stamp) bool { - return checkObserved(fs, Observed.of(current)); + var fresh: StartState = .{}; + return checkObserved(fs, Observed.of(current), &fresh); } /// The launch-time check. `fs` provides: /// * `read(name: []const u8, buf: []u8) ?[]const u8` (null = absent or /// unreadable) /// * `write(name: []const u8, bytes: []const u8) bool` +/// * `rename(from: []const u8, to: []const u8) bool` (replaces `to`) /// * `delete(name: []const u8) void` (absent is fine) /// -/// Returns true when this launch must not use Vulkan. Side effects, in order: -/// 1. `.labelle_vulkan_disabled` present: matching stamp → true (and a -/// leftover start mark is dropped); an observed change (or a garbled -/// stamp) → reset (both files go), false. -/// 2. `.labelle_vulkan_start` present (the previous Vulkan start did not -/// complete): matching stamp → write `.labelle_vulkan_disabled`, drop the -/// start mark, true; an observed change (or garbled) → reset, false. If -/// the write fails, the start mark is KEPT (fail closed): the next -/// launch finds it again and is still guarded. -/// 3. Neither → false. -/// A mark whose stamp cannot be compared because the version or setting -/// could not be READ (`unknown`) is PRESERVED untouched and this launch is -/// treated as guarded (true): the next launch that reads both decides. -pub fn checkObserved(fs: anytype, current: Observed) bool { +/// Returns true when this launch must not use Vulkan. Each marker is judged +/// on its own stamp (`compare`): +/// * `.labelle_vulkan_disabled`: same → guarded; changed → deleted (only +/// it); unknown (a value could not be read) → kept, guarded; malformed → +/// guarded and rewritten with the current stamp (fail closed). +/// * `.labelle_vulkan_start`, unless this process owns it +/// (`state.owned`: a same-process relaunch, not a crash): same (or +/// malformed) → a crashed start: write the disabled mark, then drop the +/// start mark, guarded; if that write fails the start mark is KEPT, so +/// the next launch is still guarded; changed → deleted (only it); +/// unknown → kept, guarded. +/// Runs under `state.lock`, like `beginStart` and `settle`'s unlink. +pub fn checkObserved(fs: anytype, current: Observed, state: *StartState) bool { + state.acquire(); + defer state.release(); + var guarded = false; var buf: [stamp_cap]u8 = undefined; if (fs.read(disabled_file, &buf)) |bytes| { - const s = Stamp.parse(bytes) orelse { - reset(fs); - return false; - }; - switch (compare(s, current)) { - .same => { - fs.delete(start_file); - return true; - }, - .unknown => return true, - .changed => { - reset(fs); - return false; - }, + if (Stamp.parse(bytes)) |s| switch (compare(s, current)) { + .same, .unknown => guarded = true, + .changed => fs.delete(disabled_file), + } else { + // Malformed: never a reset (that could lose a kept start mark). + // Fail closed and repair it for the current stamp. + std.log.warn("android: crash guard: malformed {s}; keeping the guard", .{disabled_file}); + guarded = true; + if (current.full()) |cur| _ = writeStamp(fs, disabled_file, cur); } } - if (fs.read(start_file, &buf)) |bytes| { - const s = Stamp.parse(bytes) orelse { - reset(fs); - return false; - }; - switch (compare(s, current)) { + if (!state.owned) if (fs.read(start_file, &buf)) |bytes| { + const parsed = Stamp.parse(bytes); + const m: Match = if (parsed) |s| compare(s, current) else .same; + switch (m) { .same => { - var out: [stamp_cap]u8 = undefined; - const written = if (s.format(&out)) |text| fs.write(disabled_file, text) else false; - if (written) { - fs.delete(start_file); - } else { - // Fail closed: keep the start mark so the NEXT launch is - // still guarded. This launch is off Vulkan either way. - std.log.warn("android: crash guard: could not write {s}; keeping {s}", .{ disabled_file, start_file }); + // A start that never became stable, left by a process that is + // gone (not this one: `state.owned` is false). + guarded = true; + if (current.full()) |cur| { + if (writeStamp(fs, disabled_file, cur)) { + fs.delete(start_file); + } else { + // Fail closed: keep the start mark so the NEXT launch + // is still guarded. This launch is off Vulkan anyway. + std.log.warn("android: crash guard: could not write {s}; keeping {s}", .{ disabled_file, start_file }); + } } - return true; - }, - .unknown => return true, - .changed => { - reset(fs); - return false; }, + .unknown => guarded = true, + .changed => fs.delete(start_file), } + }; + return guarded; +} + +/// Write `.tmp`, then rename it over `name`: a failed or partial write +/// never leaves a malformed `name` (the tmp is removed on failure). +fn writeAtomic(fs: anytype, name: []const u8, bytes: []const u8) bool { + var tmp_buf: [64]u8 = undefined; + const tmp = std.fmt.bufPrint(&tmp_buf, "{s}.tmp", .{name}) catch return false; + if (!fs.write(tmp, bytes) or !fs.rename(tmp, name)) { + fs.delete(tmp); + return false; } - return false; + return true; } -fn reset(fs: anytype) void { - fs.delete(start_file); - fs.delete(disabled_file); +fn writeStamp(fs: anytype, name: []const u8, stamp: Stamp) bool { + var out: [stamp_cap]u8 = undefined; + const text = stamp.format(&out) orelse return false; + return writeAtomic(fs, name, text); } -/// Write `.labelle_vulkan_start` (called just before a Vulkan start). +/// Write `.labelle_vulkan_start` (atomically; called just before a Vulkan +/// start). pub fn markStart(fs: anytype, current: Stamp) bool { - var out: [stamp_cap]u8 = undefined; - const text = current.format(&out) orelse return false; - return fs.write(start_file, text); + return writeStamp(fs, start_file, current); } -/// The start mark's ownership, shared by `beginStart` and every stable -/// thread of the process. `lock` serialises the two steps that must not -/// interleave: -/// * `beginStart`: { generation += 1; write the start mark } -/// * `settle`'s final step: { generation still == mine?; unlink the mark } +/// The start mark's ownership, shared by `checkObserved`, `beginStart` and +/// every stable thread of the process. `lock` serialises the steps that +/// must not interleave: +/// * `beginStart`: { generation += 1; write the start mark; owned = true } +/// * `settle`'s final step: { generation still == mine?; unlink the mark; +/// owned = false } +/// * `checkObserved`: { read `owned`; judge the markers } /// so an old stable thread can never unlink a mark a newer launch just /// wrote (Codex review of labelle-android#33, labelle-bgfx#172 comment -/// 5904658178). The frame counter and the clock are read OUTSIDE the lock. +/// 5904658178), and a same-process relaunch never mistakes this process's +/// own live mark for a crash. The frame counter and the clock are read +/// OUTSIDE the lock. /// /// `std.atomic.Mutex` spun on `tryLock` (Zig 0.16 has no `std.Thread.Mutex`, -/// and `std.Io.Mutex` needs an `Io`): both critical sections are a few -/// syscalls on a tiny file and contention needs two activity launches in +/// and `std.Io.Mutex` needs an `Io`): the critical sections are a few +/// syscalls on tiny files and contention needs two activity launches in /// the same instant, so a spin is fine. pub const StartState = struct { lock: std.atomic.Mutex = .unlocked, /// Written only under `lock`; read without it (atomically) only for the /// stable loop's early exit, which is advisory. generation: u32 = 0, + /// Under `lock`: the start mark on disk was written by THIS process and + /// its stable thread has not cleared it yet. + owned: bool = false, fn acquire(self: *StartState) void { while (!self.lock.tryLock()) std.atomic.spinLoopHint(); @@ -235,33 +266,52 @@ pub const StartState = struct { } }; -/// `beginVulkanStart`'s pure half: under `state.lock`, bump the generation -/// and write the start mark; then start the stable thread via -/// `spawner.spawn(mine: u32) !void`. Fails CLOSED: if the thread cannot -/// start, the mark is KEPT, so the next launch runs on GLES rather than an -/// unguarded Vulkan start being forgotten. -pub fn beginStart(fs: anytype, current: Stamp, state: *StartState, spawner: anytype) void { +/// `beginVulkanStart`'s pure half. Reads the frame counter as this start's +/// baseline (`src.framesPresented()`, outside the lock); under `state.lock` +/// bumps the generation and writes the start mark; then starts the stable +/// thread via `spawner.spawn(mine: u32, baseline: ?u64) !void`. +/// +/// Returns false when the start mark could NOT be written: the caller then +/// must not start Vulkan unguarded (`renderer.afterStart` → GLES). If only +/// the thread cannot start, the mark is KEPT and this returns true: Vulkan +/// runs, and the next launch is guarded (fail closed). +pub fn beginStart(fs: anytype, current: Stamp, state: *StartState, src: anytype, spawner: anytype) bool { + const baseline = src.framesPresented(); state.acquire(); const mine = state.generation +% 1; @atomicStore(u32, &state.generation, mine, .release); const written = markStart(fs, current); + if (written) state.owned = true; state.release(); if (!written) { std.log.warn("android: crash guard: could not write {s}", .{start_file}); - return; + return false; } - spawner.spawn(mine) catch |err| { + spawner.spawn(mine, baseline) catch |err| { std.log.warn("android: crash guard: could not start the stable timer ({s}); keeping {s}, so the next launch uses gles", .{ @errorName(err), start_file }); }; + return true; +} + +/// Frames presented since this start: the counter minus `baseline.*`. A +/// counter BELOW the baseline means bgfx re-initialised (labelle-bgfx#182 +/// resets it on every successful init): the baseline drops to 0 for good. +fn framesSince(raw: u64, baseline: *?u64) u64 { + const b = baseline.* orelse return raw; + if (raw < b) { + baseline.* = 0; + return raw; + } + return raw - b; } /// The stable thread. Polls every `poll_interval_ns` and deletes the start -/// mark once `stable_after_ns` has passed since it started AND -/// `src.framesPresented()` is at least `stable_min_frames`. A newer -/// `beginStart` (another activity launch in the same process) takes the mark -/// over: this thread then stops without deleting. The final check-and-unlink -/// runs under `state.lock`, so it cannot interleave with a newer -/// `beginStart`'s bump-and-write. +/// mark once `stable_after_ns` has passed since it started AND at least +/// `stable_min_frames` frames were presented since `baseline` (the counter +/// at `beginStart`; see `framesSince`). A newer `beginStart` (another +/// activity launch in the same process) takes the mark over: this thread +/// then stops without deleting. The final check-and-unlink runs under +/// `state.lock`, so it cannot interleave with a newer `beginStart`. /// /// Injected: /// * `clock.now() u64` (monotonic ns) and `clock.sleep(ns) !void`; a fake @@ -269,22 +319,26 @@ pub fn beginStart(fs: anytype, current: Stamp, state: *StartState, spawner: anyt /// * `src.framesPresented() ?u64`: null = the labelle-bgfx symbol is /// missing → time-only rule; then `src.noteTimeOnly()` is called once /// (the glue logs once per process). -pub fn settle(fs: anytype, clock: anytype, src: anytype, state: *StartState, mine: u32) void { +pub fn settle(fs: anytype, clock: anytype, src: anytype, state: *StartState, mine: u32, baseline: ?u64) void { const t0 = clock.now(); var noted = false; + var base = baseline; // Advisory early exit; the authoritative check is under the lock below. while (state.peek() == mine) { - const frames = src.framesPresented(); - if (frames == null and !noted) { + const raw = src.framesPresented(); + if (raw == null and !noted) { noted = true; src.noteTimeOnly(); } const elapsed = clock.now() -% t0; - const enough_frames = if (frames) |f| f >= stable_min_frames else true; + const enough_frames = if (raw) |r| framesSince(r, &base) >= stable_min_frames else true; if (elapsed >= stable_after_ns and enough_frames) { state.acquire(); defer state.release(); - if (state.generation == mine) fs.delete(start_file); + if (state.generation == mine) { + fs.delete(start_file); + state.owned = false; + } return; } clock.sleep(poll_interval_ns) catch return; @@ -295,6 +349,7 @@ pub fn settle(fs: anytype, clock: anytype, src: anytype, state: *StartState, min // `jni/renderer_query.c`. extern "c" fn labelle_android_internal_data_path(activity: ?*const anyopaque) ?[*:0]const u8; +extern "c" fn labelle_android_no_backup_dir(activity: ?*const anyopaque, buf: [*]u8, buf_cap: usize) c_int; extern "c" fn labelle_android_version_code(activity: ?*const anyopaque, out: *c_longlong) c_int; extern "c" fn labelle_android_frames_presented(out: *u64) c_int; extern "c" fn labelle_android_monotonic_ns() u64; @@ -305,9 +360,10 @@ extern "c" fn fread(ptr: [*]u8, size: usize, n: usize, f: *FILE) usize; extern "c" fn fwrite(ptr: [*]const u8, size: usize, n: usize, f: *FILE) usize; extern "c" fn fclose(f: *FILE) c_int; extern "c" fn unlink(name: [*:0]const u8) c_int; +extern "c" fn rename(from: [*:0]const u8, to: [*:0]const u8) c_int; extern "c" fn usleep(usec: c_uint) c_int; -/// Marker files under a copied `internalDataPath` (a value type, so the +/// Marker files under a copied directory path (a value type, so the /// detached stable thread owns its own copy). const LibcFs = struct { dir_buf: [512]u8 = undefined, @@ -338,11 +394,19 @@ const LibcFs = struct { const ok = fwrite(bytes.ptr, 1, bytes.len, f) == bytes.len; return fclose(f) == 0 and ok; } + pub fn rename(self: *const LibcFs, from: []const u8, to: []const u8) bool { + var pf: [600]u8 = undefined; + var pt: [600]u8 = undefined; + const f = self.full(from, &pf) orelse return false; + const t = self.full(to, &pt) orelse return false; + return rename_c(f.ptr, t.ptr) == 0; + } pub fn delete(self: *const LibcFs, name: []const u8) void { var p: [600]u8 = undefined; _ = unlink((self.full(name, &p) orelse return).ptr); } }; +const rename_c = rename; const LibcClock = struct { pub fn now(_: LibcClock) u64 { @@ -401,7 +465,19 @@ fn versionCode(activity: *const anyopaque) ?i64 { return v; } +var no_backup_failure_logged: std.atomic.Value(bool) = .init(false); + +/// The markers' directory: `getNoBackupFilesDir()` (never backed up or +/// transferred), else `internalDataPath` with a one-time warning. fn libcFs(activity: *const anyopaque) ?LibcFs { + var nb: [512]u8 = undefined; + const n = labelle_android_no_backup_dir(activity, &nb, nb.len); + if (n > 0) { + if (LibcFs.init(nb[0..@intCast(n)])) |fs| return fs; + } + if (logOnce(&no_backup_failure_logged)) { + std.log.warn("android: crash guard: could not get noBackupFilesDir; using internalDataPath (Auto Backup may copy the crash-guard markers)", .{}); + } const dir = labelle_android_internal_data_path(activity) orelse { std.log.warn("android: crash guard: no internalDataPath; guard off", .{}); return null; @@ -423,7 +499,7 @@ pub fn vulkanDisabled(activity: ?*const anyopaque, setting: ?[]const u8) bool { if (comptime !is_android) return false; const a = activity orelse return false; var fs = libcFs(a) orelse return false; - return checkObserved(&fs, .{ .version_code = versionCode(a), .setting = setting }); + return checkObserved(&fs, .{ .version_code = versionCode(a), .setting = setting }, &start_state); } /// Called by `renderer.resolve` once Vulkan is chosen, before bgfx starts: @@ -431,47 +507,55 @@ pub fn vulkanDisabled(activity: ?*const anyopaque, setting: ?[]const u8) bool { /// (120 frames and 10 s, or 10 s alone without labelle-bgfx#182). /// `setting` null = the meta-data could not be read. A start mark needs the /// full stamp, so with either half unreadable no mark is written (logged). -pub fn beginVulkanStart(activity: ?*const anyopaque, setting: ?[]const u8) void { - if (comptime !is_android) return; - const a = activity orelse return; - const fs = libcFs(a) orelse return; +/// Returns whether the start mark was recorded; when it was not, +/// `renderer.afterStart` keeps this launch off Vulkan (fail closed). +pub fn beginVulkanStart(activity: ?*const anyopaque, setting: ?[]const u8) bool { + if (comptime !is_android) return true; + const a = activity orelse return false; + const fs = libcFs(a) orelse return false; const obs: Observed = .{ .version_code = versionCode(a), .setting = setting }; const stamp = obs.full() orelse { - std.log.warn("android: crash guard: versionCode or {s} unreadable; no start mark for this Vulkan start", .{"labelle.renderer"}); - return; + std.log.warn("android: crash guard: versionCode or {s} unreadable; cannot record this Vulkan start", .{"labelle.renderer"}); + return false; }; - beginStart(&fs, stamp, &start_state, ThreadSpawner{ .fs = fs }); + return beginStart(&fs, stamp, &start_state, DlsymFrames{}, ThreadSpawner{ .fs = fs }); } /// Spawns the detached stable-timer thread with its own copy of the fs. const ThreadSpawner = struct { fs: LibcFs, - pub fn spawn(self: ThreadSpawner, mine: u32) std.Thread.SpawnError!void { - const t = try std.Thread.spawn(.{}, stableThread, .{ self.fs, mine }); + pub fn spawn(self: ThreadSpawner, mine: u32, baseline: ?u64) std.Thread.SpawnError!void { + const t = try std.Thread.spawn(.{}, stableThread, .{ self.fs, mine, baseline }); t.detach(); } }; -fn stableThread(fs: LibcFs, mine: u32) void { - settle(&fs, LibcClock{}, DlsymFrames{}, &start_state, mine); +fn stableThread(fs: LibcFs, mine: u32, baseline: ?u64) void { + settle(&fs, LibcClock{}, DlsymFrames{}, &start_state, mine, baseline); } // ── Tests ─────────────────────────────────────────────────────────────── const testing = std.testing; -/// In-memory marker files, persisting across "launches" like internal -/// storage does. +/// In-memory marker files (and their `.tmp` siblings), persisting across +/// "launches" like storage does. const FakeFs = struct { start: ?[]const u8 = null, disabled: ?[]const u8 = null, - start_buf: [stamp_cap]u8 = undefined, - disabled_buf: [stamp_cap]u8 = undefined, + start_tmp: ?[]const u8 = null, + disabled_tmp: ?[]const u8 = null, + bufs: [4][stamp_cap]u8 = undefined, + /// Every write fails without touching anything. fail_writes: bool = false, + /// Every write stores HALF the bytes, then reports failure (ENOSPC). + partial_writes: bool = false, fn slot(self: *FakeFs, name: []const u8) struct { *?[]const u8, []u8 } { - if (std.mem.eql(u8, name, start_file)) return .{ &self.start, &self.start_buf }; - if (std.mem.eql(u8, name, disabled_file)) return .{ &self.disabled, &self.disabled_buf }; + if (std.mem.eql(u8, name, start_file)) return .{ &self.start, &self.bufs[0] }; + if (std.mem.eql(u8, name, disabled_file)) return .{ &self.disabled, &self.bufs[1] }; + if (std.mem.eql(u8, name, start_file ++ ".tmp")) return .{ &self.start_tmp, &self.bufs[2] }; + if (std.mem.eql(u8, name, disabled_file ++ ".tmp")) return .{ &self.disabled_tmp, &self.bufs[3] }; @panic("unexpected file name"); } pub fn read(self: *FakeFs, name: []const u8, buf: []u8) ?[]const u8 { @@ -483,18 +567,27 @@ const FakeFs = struct { pub fn write(self: *FakeFs, name: []const u8, bytes: []const u8) bool { if (self.fail_writes) return false; const s = self.slot(name); - @memcpy(s[1][0..bytes.len], bytes); - s[0].* = s[1][0..bytes.len]; + const n = if (self.partial_writes) bytes.len / 2 else bytes.len; + @memcpy(s[1][0..n], bytes[0..n]); + s[0].* = s[1][0..n]; + return !self.partial_writes; + } + pub fn rename(self: *FakeFs, from: []const u8, to: []const u8) bool { + const f = self.slot(from); + const v = f[0].* orelse return false; + const t = self.slot(to); + @memcpy(t[1][0..v.len], v); + t[0].* = t[1][0..v.len]; + f[0].* = null; return true; } pub fn delete(self: *FakeFs, name: []const u8) void { self.slot(name)[0].* = null; } fn set(self: *FakeFs, name: []const u8, bytes: []const u8) void { - const saved = self.fail_writes; - self.fail_writes = false; - _ = self.write(name, bytes); - self.fail_writes = saved; + const s = self.slot(name); + @memcpy(s[1][0..bytes.len], bytes); + s[0].* = s[1][0..bytes.len]; } }; @@ -517,13 +610,16 @@ const FakeClock = struct { } }; -/// labelle-bgfx#182's counter as a function of the fake clock: 0 until -/// `init_done_ns` (null = init never finishes, i.e. hangs), then `fps` -/// frames per second. `missing` = the symbol is not exported. +/// labelle-bgfx#182's counter as a function of the fake clock: +/// `before_init` until `init_done_ns` (null = init never finishes, i.e. +/// hangs; `before_init` > 0 = a previous Activity's frames), then from 0 at +/// `fps` frames per second (the reset on a successful init). `missing` = the +/// symbol is not exported. const FakeFrames = struct { clock: *FakeClock, fps: u64 = 60, init_done_ns: ?u64 = 0, + before_init: u64 = 0, missing: bool = false, notes: usize = 0, calls: usize = 0, @@ -535,11 +631,11 @@ const FakeFrames = struct { pub fn framesPresented(self: *FakeFrames) ?u64 { self.calls += 1; if (self.restart) |r| if (self.calls == r.at_call) { - beginStart(r.fs, r.stamp, r.state, NoSpawner{}); + _ = beginStart(r.fs, r.stamp, r.state, NoFrames{}, NoSpawner{}); }; if (self.missing) return null; - const done = self.init_done_ns orelse return 0; - if (self.clock.now_ns < done) return 0; + const done = self.init_done_ns orelse return self.before_init; + if (self.clock.now_ns < done) return self.before_init; return (self.clock.now_ns - done) * self.fps / std.time.ns_per_s; } pub fn noteTimeOnly(self: *FakeFrames) void { @@ -548,9 +644,23 @@ const FakeFrames = struct { }; const NoSpawner = struct { - pub fn spawn(_: NoSpawner, _: u32) error{}!void {} + pub fn spawn(_: NoSpawner, _: u32, _: ?u64) error{}!void {} }; +/// A frame source with no counter (for `beginStart`'s baseline read). +const NoFrames = struct { + pub fn framesPresented(_: NoFrames) ?u64 { + return null; + } + pub fn noteTimeOnly(_: NoFrames) void {} +}; + +/// `checkObserved` as a NEW process (fresh in-process state). +fn checkNew(fs: anytype, obs: Observed) bool { + var fresh: StartState = .{}; + return checkObserved(fs, obs, &fresh); +} + const v1_vulkan: Stamp = .{ .version_code = 1, .setting = "vulkan" }; const v2_vulkan: Stamp = .{ .version_code = 2, .setting = "vulkan" }; @@ -559,24 +669,26 @@ const v2_vulkan: Stamp = .{ .version_code = 2, .setting = "vulkan" }; fn launchVulkan(fs: *FakeFs, clock: *FakeClock, state: *StartState, stamp: Stamp) bool { if (checkAtLaunch(fs, stamp)) return false; var got: ?u32 = null; - beginStart(fs, stamp, state, RecordingSpawner{ .got = &got }); + _ = beginStart(fs, stamp, state, NoFrames{}, RecordingSpawner{ .got = &got }); var frames: FakeFrames = .{ .clock = clock }; - if (got) |mine| settle(fs, clock, &frames, state, mine); + if (got) |mine| settle(fs, clock, &frames, state, mine, null); return true; } const RecordingSpawner = struct { got: *?u32, - pub fn spawn(self: RecordingSpawner, mine: u32) error{}!void { + baseline: ?*?u64 = null, + pub fn spawn(self: RecordingSpawner, mine: u32, baseline: ?u64) error{}!void { self.got.* = mine; + if (self.baseline) |b| b.* = baseline; } }; /// Mark a start and run only the stable thread; returns the frame fake. fn runSettle(fs: *FakeFs, clock: *FakeClock, frames: *FakeFrames) void { var state: StartState = .{}; - beginStart(fs, v1_vulkan, &state, NoSpawner{}); - settle(fs, clock, frames, &state, state.generation); + _ = beginStart(fs, v1_vulkan, &state, NoFrames{}, NoSpawner{}); + settle(fs, clock, frames, &state, state.generation, null); } test "stamp: format and parse round-trip; junk is rejected" { @@ -638,7 +750,7 @@ test "a failed disabled write keeps the start mark (fail closed): still guarded test "a stable timer that cannot start keeps the start mark (fail closed)" { const FailingSpawner = struct { calls: *usize, - pub fn spawn(self: @This(), _: u32) error{SystemResources}!void { + pub fn spawn(self: @This(), _: u32, _: ?u64) error{SystemResources}!void { self.calls.* += 1; return error.SystemResources; } @@ -646,7 +758,7 @@ test "a stable timer that cannot start keeps the start mark (fail closed)" { var fs: FakeFs = .{}; var state: StartState = .{}; var calls: usize = 0; - beginStart(&fs, v1_vulkan, &state, FailingSpawner{ .calls = &calls }); + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, FailingSpawner{ .calls = &calls })); // mark written try testing.expectEqual(@as(usize, 1), calls); try testing.expectEqualStrings("1 vulkan", fs.start.?); // kept // The next launch treats it as an incomplete start: gles. @@ -658,7 +770,7 @@ test "beginStart: mark written, timer spawned with the new generation" { var fs: FakeFs = .{}; var state: StartState = .{ .generation = 4 }; var got: ?u32 = null; - beginStart(&fs, v1_vulkan, &state, RecordingSpawner{ .got = &got }); + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, RecordingSpawner{ .got = &got })); try testing.expectEqual(@as(?u32, 5), got); try testing.expectEqualStrings("1 vulkan", fs.start.?); } @@ -677,12 +789,12 @@ test "surviving 10 s (with frames) clears the mark; the next launch tries Vulkan test "a superseded stable timer leaves the newer start mark alone" { var fs: FakeFs = .{}; var state: StartState = .{}; - beginStart(&fs, v1_vulkan, &state, NoSpawner{}); + _ = beginStart(&fs, v1_vulkan, &state, NoFrames{}, NoSpawner{}); const first = state.generation; var clock: FakeClock = .{}; // A second beginStart lands on the 3rd poll, well before 10 s. var frames: FakeFrames = .{ .clock = &clock, .restart = .{ .at_call = 3, .fs = &fs, .state = &state, .stamp = v2_vulkan } }; - settle(&fs, &clock, &frames, &state, first); + settle(&fs, &clock, &frames, &state, first, null); try testing.expectEqual(@as(usize, 3), frames.calls); // it polled... try testing.expect(clock.now_ns < stable_after_ns); // ...stopped early... try testing.expectEqualStrings("2 vulkan", fs.start.?); // ...and left the new mark @@ -697,11 +809,11 @@ test "race: a newer start landing at the very poll that would clear keeps its ma // fails: fs.start == null). var fs: FakeFs = .{}; var state: StartState = .{}; - beginStart(&fs, v1_vulkan, &state, NoSpawner{}); + _ = beginStart(&fs, v1_vulkan, &state, NoFrames{}, NoSpawner{}); const first = state.generation; var clock: FakeClock = .{}; var frames: FakeFrames = .{ .clock = &clock, .restart = .{ .at_call = 21, .fs = &fs, .state = &state, .stamp = v2_vulkan } }; - settle(&fs, &clock, &frames, &state, first); + settle(&fs, &clock, &frames, &state, first, null); try testing.expectEqual(@as(usize, 21), frames.calls); try testing.expectEqual(stable_after_ns, clock.now_ns); // it reached the stable branch try testing.expectEqual(first + 1, state.generation); @@ -716,11 +828,11 @@ test "the stable thread's own clear still works under the lock" { var clock: FakeClock = .{}; var frames: FakeFrames = .{ .clock = &clock }; var state: StartState = .{}; - beginStart(&fs, v1_vulkan, &state, NoSpawner{}); + _ = beginStart(&fs, v1_vulkan, &state, NoFrames{}, NoSpawner{}); try testing.expect(state.lock.tryLock()); // beginStart released it... try testing.expect(!state.lock.tryLock()); // ...and it is exclusive state.release(); - settle(&fs, &clock, &frames, &state, state.generation); + settle(&fs, &clock, &frames, &state, state.generation, null); try testing.expect(fs.start == null); try testing.expect(state.lock.tryLock()); state.release(); @@ -823,14 +935,17 @@ test "a changed renderer setting resets both files" { try testing.expect(fs.start == null and fs.disabled == null); } -test "an unreadable stamp resets" { +test "a malformed marker is never a reset: fail closed, repaired for the current stamp" { var fs: FakeFs = .{}; fs.set(disabled_file, "garbage"); - try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); - try testing.expect(fs.disabled == null); - fs.set(start_file, ""); - try testing.expect(!checkAtLaunch(&fs, v1_vulkan)); - try testing.expect(fs.start == null); + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); + + var fs2: FakeFs = .{}; + fs2.set(start_file, ""); + try testing.expect(checkAtLaunch(&fs2, v1_vulkan)); // an unfinished start + try testing.expect(fs2.start == null); + try testing.expectEqualStrings("1 vulkan", fs2.disabled.?); } test "a disabled launch drops a leftover start mark" { @@ -895,7 +1010,7 @@ test "the Android entry points are no-ops off Android" { if (comptime is_android) return error.SkipZigTest; try testing.expect(!vulkanDisabled(null, "vulkan")); try testing.expect(!vulkanDisabled(@ptrFromInt(0x1000), "vulkan")); - beginVulkanStart(@ptrFromInt(0x1000), "vulkan"); + try testing.expect(beginVulkanStart(@ptrFromInt(0x1000), "vulkan")); } test "Android: the glue is analysed (compile-check only; nothing runs)" { @@ -924,7 +1039,7 @@ const LaunchQuery = struct { return false; } pub fn vulkanDisabled(self: *@This()) bool { - return checkObserved(self.fs, .{ .version_code = self.version, .setting = self.meta.guardSetting() }); + return checkNew(self.fs, .{ .version_code = self.version, .setting = self.meta.guardSetting() }); } pub fn metaData(self: *@This()) ?[]const u8 { return self.meta.metaData(); @@ -988,7 +1103,7 @@ test "a failed versionCode read preserves the marks; the next good read is still // Same for a pending start mark. var fs2: FakeFs = .{}; fs2.set(start_file, "42 vulkan"); - try testing.expect(checkObserved(&fs2, .{ .version_code = null, .setting = "vulkan" })); + try testing.expect(checkNew(&fs2, .{ .version_code = null, .setting = "vulkan" })); try testing.expectEqualStrings("42 vulkan", fs2.start.?); try testing.expect(fs2.disabled == null); const d3 = LaunchQuery.run(&fs2, .{ .value = "vulkan" }, 42); @@ -1000,11 +1115,11 @@ test "an observed change still resets even when the OTHER half is unreadable" { var fs: FakeFs = .{}; fs.set(disabled_file, "1 vulkan"); // A successfully read new version, setting unreadable: a real change. - try testing.expect(!checkObserved(&fs, .{ .version_code = 2, .setting = null })); + try testing.expect(!checkNew(&fs, .{ .version_code = 2, .setting = null })); try testing.expect(fs.disabled == null); fs.set(disabled_file, "1 vulkan"); // A successfully read different setting, version unreadable. - try testing.expect(!checkObserved(&fs, .{ .version_code = null, .setting = "gles" })); + try testing.expect(!checkNew(&fs, .{ .version_code = null, .setting = "gles" })); try testing.expect(fs.disabled == null); } @@ -1018,3 +1133,162 @@ test "an absent key is a successful read of the default gles: a real change vs a try testing.expectEqual(R.Source.setting, d.source); // not the guard try testing.expect(fs.disabled == null and fs.start == null); // reset } + +// ── Codex bot findings on labelle-android#33 (final round) ───────────── + +test "same-process relaunch before stable supersedes its own mark (no disable)" { + var fs: FakeFs = .{}; + var state: StartState = .{}; // ONE process + try testing.expect(!checkObserved(&fs, Observed.of(v1_vulkan), &state)); + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, NoSpawner{})); + const first = state.generation; + try testing.expect(state.owned); + + // The Activity is relaunched in the same process 3 s later: its own live + // mark is not a crash. + try testing.expect(!checkObserved(&fs, Observed.of(v1_vulkan), &state)); + try testing.expect(fs.disabled == null); + try testing.expectEqualStrings("1 vulkan", fs.start.?); + // It supersedes: new generation, new mark. + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, NoSpawner{})); + try testing.expectEqual(first + 1, state.generation); + + // The old timer, even when stable, cannot delete the new mark. + var clock: FakeClock = .{}; + var frames: FakeFrames = .{ .clock = &clock }; + settle(&fs, &clock, &frames, &state, first, null); + try testing.expectEqualStrings("1 vulkan", fs.start.?); + try testing.expect(state.owned); + // The new one can, and gives up ownership. + var clock2: FakeClock = .{}; + var frames2: FakeFrames = .{ .clock = &clock2 }; + settle(&fs, &clock2, &frames2, &state, first + 1, null); + try testing.expect(fs.start == null); + try testing.expect(!state.owned); +} + +test "a mark left by a different (dead) process is still a crash" { + var fs: FakeFs = .{}; + var dead: StartState = .{}; + try testing.expect(beginStart(&fs, v1_vulkan, &dead, NoFrames{}, NoSpawner{})); + // The process dies; a NEW process (fresh state) launches. + var next: StartState = .{}; + try testing.expect(checkObserved(&fs, Observed.of(v1_vulkan), &next)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); + try testing.expect(fs.start == null); +} + +test "frame baseline: a hung new start is not cleared by the previous Activity's frames" { + var fs: FakeFs = .{}; + var state: StartState = .{}; + var clock: FakeClock = .{ .dies_at_ns = 60 * std.time.ns_per_s }; + // The previous Activity presented 600 frames; the new init hangs, so the + // counter stays at 600 (never reset). + var frames: FakeFrames = .{ .clock = &clock, .init_done_ns = null, .before_init = 600 }; + var got: ?u32 = null; + var baseline: ?u64 = null; + try testing.expect(beginStart(&fs, v1_vulkan, &state, &frames, RecordingSpawner{ .got = &got, .baseline = &baseline })); + try testing.expectEqual(@as(?u64, 600), baseline); + settle(&fs, &clock, &frames, &state, got.?, baseline); + try testing.expect(clock.now_ns >= stable_after_ns); + try testing.expect(fs.start != null); // not cleared: the process is killed with the mark +} + +test "frame baseline: a counter reset by the new init counts from 0" { + var fs: FakeFs = .{}; + var state: StartState = .{}; + var clock: FakeClock = .{}; + // 600 old frames; the new init completes at 1 s (counter → 0), then 60 fps. + var frames: FakeFrames = .{ .clock = &clock, .init_done_ns = std.time.ns_per_s, .before_init = 600 }; + var got: ?u32 = null; + var baseline: ?u64 = null; + try testing.expect(beginStart(&fs, v1_vulkan, &state, &frames, RecordingSpawner{ .got = &got, .baseline = &baseline })); + settle(&fs, &clock, &frames, &state, got.?, baseline); + try testing.expect(fs.start == null); + try testing.expectEqual(stable_after_ns, clock.now_ns); // 540 new frames by then + + // framesSince itself: under the baseline = reset (sticky), else the delta. + var b: ?u64 = 600; + try testing.expectEqual(@as(u64, 0), framesSince(600, &b)); + try testing.expectEqual(@as(u64, 10), framesSince(610, &b)); + try testing.expectEqual(@as(u64, 5), framesSince(5, &b)); + try testing.expectEqual(@as(?u64, 0), b); + try testing.expectEqual(@as(u64, 700), framesSince(700, &b)); + var none: ?u64 = null; + try testing.expectEqual(@as(u64, 42), framesSince(42, &none)); +} + +test "a version change deletes only the mismatching marker; a matching start mark still counts" { + // v1 disabled (stale); an intent launch on v2 wrote a v2 start mark and + // crashed. The next ordinary v2 launch must still be guarded. + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + fs.set(start_file, "2 vulkan"); + try testing.expect(checkAtLaunch(&fs, v2_vulkan)); + try testing.expectEqualStrings("2 vulkan", fs.disabled.?); // rewritten for v2 + try testing.expect(fs.start == null); + + // A stale start mark with a still-matching disabled mark: only the start + // mark goes. + var fs2: FakeFs = .{}; + fs2.set(disabled_file, "2 vulkan"); + fs2.set(start_file, "1 vulkan"); + try testing.expect(checkAtLaunch(&fs2, v2_vulkan)); + try testing.expectEqualStrings("2 vulkan", fs2.disabled.?); + try testing.expect(fs2.start == null); +} + +test "a malformed disabled mark keeps the guard and does not lose a kept start mark" { + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vul"); // a partial write from an older build + fs.set(start_file, "1 vulkan"); + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); // rewritten + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); // and stays guarded +} + +test "atomic writes: a partial disabled write leaves no malformed marker" { + var fs: FakeFs = .{}; + fs.set(start_file, "1 vulkan"); + fs.partial_writes = true; // ENOSPC mid-write + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expect(fs.disabled == null); // never a truncated marker + try testing.expect(fs.disabled_tmp == null); // the tmp is cleaned up + try testing.expectEqualStrings("1 vulkan", fs.start.?); // kept (fail closed) + // Space comes back: the next launch converts it. + fs.partial_writes = false; + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); + try testing.expect(fs.start == null); +} + +test "a start mark that cannot be written falls back to gles (unless the intent asked)" { + const R = @import("renderer.zig"); + var fs: FakeFs = .{}; + var state: StartState = .{}; + fs.partial_writes = true; + var calls: usize = 0; + const CountingSpawner = struct { + calls: *usize, + pub fn spawn(self: @This(), _: u32, _: ?u64) error{}!void { + self.calls.* += 1; + } + }; + const recorded = beginStart(&fs, v1_vulkan, &state, NoFrames{}, CountingSpawner{ .calls = &calls }); + try testing.expect(!recorded); + try testing.expect(fs.start == null and fs.start_tmp == null); + try testing.expectEqual(@as(usize, 0), calls); // no timer for no mark + try testing.expect(!state.owned); + + const d = R.afterStart(.{ .renderer = .vulkan, .source = .setting }, recorded); + try testing.expectEqual(R.Renderer.gles, d.renderer); + try testing.expectEqual(R.Source.crash_guard_unrecorded, d.source); + const a = R.afterStart(.{ .renderer = .vulkan, .source = .auto }, recorded); + try testing.expectEqual(R.Renderer.gles, a.renderer); + // The intent override keeps Vulkan; a recorded start changes nothing. + const i = R.afterStart(.{ .renderer = .vulkan, .source = .intent }, recorded); + try testing.expectEqual(R.Renderer.vulkan, i.renderer); + const ok = R.afterStart(.{ .renderer = .vulkan, .source = .setting }, true); + try testing.expectEqual(R.Renderer.vulkan, ok.renderer); + try testing.expectEqual(R.Source.setting, ok.source); +} diff --git a/src/jni/renderer_query.c b/src/jni/renderer_query.c index 249dcf7..ce6f542 100644 --- a/src/jni/renderer_query.c +++ b/src/jni/renderer_query.c @@ -165,8 +165,62 @@ int labelle_android_has_system_feature(const void *activity_ptr, const char *nam return has; } +// `activity.getNoBackupFilesDir().getAbsolutePath()` (API 21) into `buf` +// (NUL-terminated): the crash guard's marker directory (labelle-android#28), +// which Android Auto Backup and device-to-device transfer never copy. +// Returns its length (> 0), -1 on any JNI failure, -3 when it does not fit +// `buf_cap`. Never leaves a Java exception pending and never leaks a local +// ref. Callable from any thread (attached or not). +int labelle_android_no_backup_dir(const void *activity_ptr, char *buf, size_t buf_cap) { + const ANativeActivity *na = (const ANativeActivity *)activity_ptr; + if (na == NULL || na->vm == NULL || na->clazz == NULL || buf == NULL || buf_cap == 0) return -1; + JavaVM *vm = na->vm; + jobject activity = na->clazz; + + int we_attached = 0; + JNIEnv *env = labelle_android_acquire_env(vm, &we_attached); + if (env == NULL) return -1; + + int result = -1; + if ((*env)->PushLocalFrame(env, 8) == JNI_OK) { + jclass activity_cls = (*env)->GetObjectClass(env, activity); + jmethodID get_dir = (activity_cls && !(*env)->ExceptionCheck(env)) ? (*env)->GetMethodID(env, activity_cls, "getNoBackupFilesDir", "()Ljava/io/File;") : NULL; + jobject dir = (get_dir && !(*env)->ExceptionCheck(env)) ? (*env)->CallObjectMethod(env, activity, get_dir) : NULL; + jclass file_cls = (dir && !(*env)->ExceptionCheck(env)) ? (*env)->GetObjectClass(env, dir) : NULL; + jmethodID get_path = (file_cls && !(*env)->ExceptionCheck(env)) ? (*env)->GetMethodID(env, file_cls, "getAbsolutePath", "()Ljava/lang/String;") : NULL; + jstring jpath = (get_path && !(*env)->ExceptionCheck(env)) ? (jstring)(*env)->CallObjectMethod(env, dir, get_path) : NULL; + if (jpath != NULL && !(*env)->ExceptionCheck(env)) { + jsize chars = (*env)->GetStringLength(env, jpath); + jsize bytes = (*env)->GetStringUTFLength(env, jpath); + if (bytes <= 0) { + result = -1; + } else if ((size_t)bytes + 1 > buf_cap) { + result = -3; + } else { + (*env)->GetStringUTFRegion(env, jpath, 0, chars, buf); + if (!(*env)->ExceptionCheck(env)) { + buf[bytes] = 0; + result = (int)bytes; + } + } + } + if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + result = -1; + } + (*env)->PopLocalFrame(env, NULL); + } else if ((*env)->ExceptionCheck(env)) { + (*env)->ExceptionClear(env); + __android_log_print(ANDROID_LOG_WARN, "labelle-android", "no-backup dir: PushLocalFrame failed; exception cleared"); + } + + if (we_attached) (*vm)->DetachCurrentThread(vm); + return result; +} + // `ANativeActivity.internalDataPath` (the app's private files dir), or NULL. -// The crash guard (labelle-android#28) keeps its two marker files there. +// The crash guard's fallback marker directory (labelle-android#28) when +// `labelle_android_no_backup_dir` fails. const char *labelle_android_internal_data_path(const void *activity_ptr) { const ANativeActivity *na = (const ANativeActivity *)activity_ptr; return na != NULL ? na->internalDataPath : NULL; diff --git a/src/renderer.zig b/src/renderer.zig index 2072424..ea3d681 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -21,10 +21,13 @@ //! `android.hardware.vulkan.version` >= 1.1 (0x401000), else `gles`. //! //! Then, when the result is `vulkan`, `crash_guard.beginVulkanStart` (the -//! start mark + the stable thread: 120 frames and 10 s); -//! `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles", 1)`; and one log line: +//! start mark + the stable thread: 120 frames and 10 s). If the start mark +//! cannot be recorded, `afterStart` switches this launch to `gles` (fail +//! closed; not for an intent override). Then +//! `setenv("LABELLE_BGFX_RENDERER", "vulkan"|"gles", 1)` and one log line: //! `renderer: (source: intent|crash-guard|setting|auto)`, with -//! `; previous Vulkan start did not complete` after `crash-guard`. +//! `; previous Vulkan start did not complete` (or `; could not record the +//! Vulkan start`) after `crash-guard`. //! //! ## Where it runs //! @@ -65,13 +68,16 @@ pub const Renderer = enum { pub const Source = enum { intent, crash_guard, + /// The guard could not record the Vulkan start (no start mark), so this + /// launch stays off Vulkan (`afterStart`). + crash_guard_unrecorded, setting, auto, pub fn label(self: Source) []const u8 { return switch (self) { .intent => "intent", - .crash_guard => "crash-guard", + .crash_guard, .crash_guard_unrecorded => "crash-guard", .setting => "setting", .auto => "auto", }; @@ -81,6 +87,7 @@ pub const Source = enum { pub fn detail(self: Source) []const u8 { return switch (self) { .crash_guard => "; previous Vulkan start did not complete", + .crash_guard_unrecorded => "; could not record the Vulkan start", else => "", }; } @@ -88,6 +95,19 @@ pub const Source = enum { pub const Decision = struct { renderer: Renderer, source: Source }; +/// After `crash_guard.beginVulkanStart` for a `vulkan` decision: when the +/// start mark could NOT be recorded (`recorded` false), a Vulkan crash would +/// go unnoticed, so the launch falls back to `gles` (fail closed). An intent +/// override is a developer's explicit request and keeps Vulkan (warned). +pub fn afterStart(d: Decision, recorded: bool) Decision { + if (d.renderer != .vulkan or recorded) return d; + if (d.source == .intent) { + std.log.warn("android: crash guard: Vulkan start not recorded; keeping vulkan for the intent override", .{}); + return d; + } + return .{ .renderer = .gles, .source = .crash_guard_unrecorded }; +} + /// The provider setting's values. pub const Setting = enum { gles, vulkan, auto }; @@ -254,8 +274,8 @@ pub fn resolve(activity: ?*const anyopaque, intent_extra: ?[:0]const u8) void { if (comptime !is_android) return; const a = activity orelse return; var q: JniQuery = .{ .activity = a, .extra = intent_extra }; - const d = decide(&q); - if (d.renderer == .vulkan) crash_guard.beginVulkanStart(a, q.metaRead().guardSetting()); + var d = decide(&q); + if (d.renderer == .vulkan) d = afterStart(d, crash_guard.beginVulkanStart(a, q.metaRead().guardSetting())); if (setenv(env_name.ptr, d.renderer.envValue().ptr, 1) != 0) { std.log.warn("android: could not set {s}={s}", .{ env_name, d.renderer.envValue() }); } @@ -394,6 +414,8 @@ test "env values and log labels" { try testing.expectEqualStrings("setting", Source.setting.label()); try testing.expectEqualStrings("auto", Source.auto.label()); try testing.expectEqualStrings("; previous Vulkan start did not complete", Source.crash_guard.detail()); + try testing.expectEqualStrings("crash-guard", Source.crash_guard_unrecorded.label()); + try testing.expectEqualStrings("; could not record the Vulkan start", Source.crash_guard_unrecorded.detail()); try testing.expectEqualStrings("", Source.setting.detail()); } From f1436dc495b284512689e46c72c45d01d760679b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexandre=20Mondaini=20Calv=C3=A3o?= Date: Wed, 30 Sep 2026 09:42:55 -0300 Subject: [PATCH 8/8] fix(crash_guard): Codex CHANGES REQUESTED at 7c28035 (4 correctness paths) labelle-bgfx#172 comments 5909398772 / 5909522964, #33 bot threads: 1. (P1) GLES same-process relaunch: renderer.resolve calls crash_guard.beginResolution() on EVERY resolution; supersede() bumps the generation under the lock without touching the start mark, so an earlier Vulkan start's thread stops and its mark stays "incomplete". Only a Vulkan start that itself reaches 120 frames + 10 s clears it. 2. Marker reads are three-state (ReadResult: present / absent / read_error; LibcFs uses errno ENOENT and ferror). Any read error, in either dir, fails closed: guarded, nothing written or deleted. 3. fs.delete returns success (absent = success). A failed stable unlink keeps ownership and retries every poll while alive (logged once). 4. Marker dirs resolved ONCE per process (markerDirs): no_backup primary + internalDataPath secondary, or internalDataPath primary + derived /no_backup secondary after a JNI failure. checkDirs reads both; a marker in either is effective; writes go only to the primary. Host regression tests for all four, each verified to fail with its fix reverted. Claude-Session: https://claude.ai/code/session_01LpszrcSrLLijQcxxvgWyUQ --- src/crash_guard.zig | 458 ++++++++++++++++++++++++++++++++++++-------- src/renderer.zig | 3 + 2 files changed, 380 insertions(+), 81 deletions(-) diff --git a/src/crash_guard.zig b/src/crash_guard.zig index a701f06..893835f 100644 --- a/src/crash_guard.zig +++ b/src/crash_guard.zig @@ -7,7 +7,11 @@ //! `Context.getNoBackupFilesDir()` (`/no_backup`), which Android Auto //! Backup and device-to-device transfer never copy: a crash on one device's //! GPU must not disable Vulkan on another. If that JNI lookup fails, the -//! glue falls back to `ANativeActivity.internalDataPath` (logged once). +//! glue falls back to `ANativeActivity.internalDataPath` (logged). The +//! directory is resolved ONCE per process (`markerDirs`) and every write +//! goes there; the launch check ALSO reads the other location +//! (`internalDataPath`, or the derived `/no_backup` after a fallback), +//! and a marker found in either is effective (`checkDirs`). //! //! * `.labelle_vulkan_start`: written by `beginVulkanStart`, just before a //! Vulkan start (the resolved renderer is `vulkan`). @@ -37,6 +41,14 @@ //! * Same process: a start mark THIS process wrote and still owns //! (`StartState.owned`) is not a crash: an Activity relaunched in the //! same process supersedes it (new generation, new mark) instead. +//! EVERY renderer resolution (`beginResolution`, whatever it picks) +//! bumps the generation, so an earlier Vulkan start's thread stops and +//! its mark stays "incomplete": only a Vulkan start that itself reaches +//! the threshold clears it (a GLES relaunch's frames never do). +//! * Stable unlink failure: ownership is kept and the unlink retried each +//! poll while the process lives (logged once). +//! * Marker READ ERRORS (not "absent") fail closed: guarded, and nothing +//! is written or deleted. //! * Disabled: while `.labelle_vulkan_disabled` exists and its stamp //! matches the current `versionCode` and setting, `vulkanDisabled` //! returns true and `renderer.decide` picks `gles` (source @@ -137,71 +149,121 @@ fn compare(recorded: Stamp, now: Observed) Match { return if (now.full() == null) .unknown else .same; } +/// A marker read: its bytes, a known-absent file, or a READ ERROR (the file +/// may exist but could not be read). Read errors fail closed. +pub const ReadResult = union(enum) { + present: []const u8, + absent, + read_error, +}; + /// The launch-time check of a NEW process (fresh `StartState`) with a fully -/// read stamp (see `checkObserved`). +/// read stamp, in a single marker directory (see `checkDirs`). pub fn checkAtLaunch(fs: anytype, current: Stamp) bool { var fresh: StartState = .{}; return checkObserved(fs, Observed.of(current), &fresh); } -/// The launch-time check. `fs` provides: -/// * `read(name: []const u8, buf: []u8) ?[]const u8` (null = absent or -/// unreadable) +/// `checkDirs` with only the primary marker directory. +pub fn checkObserved(fs: anytype, current: Observed, state: *StartState) bool { + return checkDirs(fs, @as(?@TypeOf(fs), null), current, state); +} + +/// The launch-time check. `primary` is the marker directory this process +/// writes to; `secondary` (optional) is the OTHER possible directory (a +/// marker from an older build or a fallback launch may be there): it is +/// read, and its markers are just as effective, but never written. Each fs +/// provides: +/// * `read(name: []const u8, buf: []u8) ReadResult` /// * `write(name: []const u8, bytes: []const u8) bool` /// * `rename(from: []const u8, to: []const u8) bool` (replaces `to`) -/// * `delete(name: []const u8) void` (absent is fine) +/// * `delete(name: []const u8) bool` (true = gone, incl. already absent) /// -/// Returns true when this launch must not use Vulkan. Each marker is judged -/// on its own stamp (`compare`): -/// * `.labelle_vulkan_disabled`: same → guarded; changed → deleted (only -/// it); unknown (a value could not be read) → kept, guarded; malformed → -/// guarded and rewritten with the current stamp (fail closed). -/// * `.labelle_vulkan_start`, unless this process owns it -/// (`state.owned`: a same-process relaunch, not a crash): same (or -/// malformed) → a crashed start: write the disabled mark, then drop the -/// start mark, guarded; if that write fails the start mark is KEPT, so -/// the next launch is still guarded; changed → deleted (only it); -/// unknown → kept, guarded. +/// Returns true when this launch must not use Vulkan. +/// * Any marker READ ERROR in either directory: guarded, and NOTHING is +/// written or deleted (fail closed; the next launch re-reads). +/// * Otherwise each marker in each directory is judged on its own stamp +/// (`compare`): +/// - `.labelle_vulkan_disabled`: same → guarded; changed → deleted +/// (only it); unknown (a value could not be read) → kept, guarded; +/// malformed → guarded and rewritten in `primary` for the current +/// stamp (fail closed). +/// - `.labelle_vulkan_start`, unless it is `primary`'s and this process +/// owns it (`state.owned`: a same-process relaunch, not a crash): +/// same (or malformed) → a crashed start: write the disabled mark in +/// `primary`, then drop this start mark, guarded; if that write +/// fails the start mark is KEPT, so the next launch is still +/// guarded; changed → deleted (only it); unknown → kept, guarded. /// Runs under `state.lock`, like `beginStart` and `settle`'s unlink. -pub fn checkObserved(fs: anytype, current: Observed, state: *StartState) bool { +pub fn checkDirs(primary: anytype, secondary: ?@TypeOf(primary), current: Observed, state: *StartState) bool { state.acquire(); defer state.release(); - var guarded = false; - var buf: [stamp_cap]u8 = undefined; - if (fs.read(disabled_file, &buf)) |bytes| { - if (Stamp.parse(bytes)) |s| switch (compare(s, current)) { - .same, .unknown => guarded = true, - .changed => fs.delete(disabled_file), - } else { - // Malformed: never a reset (that could lose a kept start mark). - // Fail closed and repair it for the current stamp. - std.log.warn("android: crash guard: malformed {s}; keeping the guard", .{disabled_file}); - guarded = true; - if (current.full()) |cur| _ = writeStamp(fs, disabled_file, cur); - } + + const Dir = struct { fs: @TypeOf(primary), is_primary: bool }; + var dirs_buf: [2]Dir = undefined; + dirs_buf[0] = .{ .fs = primary, .is_primary = true }; + var n: usize = 1; + if (secondary) |sec| { + dirs_buf[1] = .{ .fs = sec, .is_primary = false }; + n = 2; } - if (!state.owned) if (fs.read(start_file, &buf)) |bytes| { - const parsed = Stamp.parse(bytes); - const m: Match = if (parsed) |s| compare(s, current) else .same; - switch (m) { - .same => { - // A start that never became stable, left by a process that is - // gone (not this one: `state.owned` is false). + const dirs = dirs_buf[0..n]; + + // Read everything first: a read error anywhere means "don't touch". + var bufs: [2][2][stamp_cap]u8 = undefined; + var reads: [2][2]ReadResult = undefined; // [dir][0 = disabled, 1 = start] + for (dirs, 0..) |d, i| { + reads[i][0] = d.fs.read(disabled_file, &bufs[i][0]); + reads[i][1] = d.fs.read(start_file, &bufs[i][1]); + for (reads[i]) |r| if (r == .read_error) { + std.log.warn("android: crash guard: could not read a marker; keeping the guard (nothing changed)", .{}); + return true; + }; + } + + var guarded = false; + for (dirs, 0..) |d, i| { + switch (reads[i][0]) { + .absent, .read_error => {}, + .present => |bytes| if (Stamp.parse(bytes)) |s| switch (compare(s, current)) { + .same, .unknown => guarded = true, + .changed => _ = d.fs.delete(disabled_file), + } else { + // Malformed: never a reset (that could lose a kept start + // mark). Fail closed and repair it for the current stamp. + std.log.warn("android: crash guard: malformed {s}; keeping the guard", .{disabled_file}); guarded = true; if (current.full()) |cur| { - if (writeStamp(fs, disabled_file, cur)) { - fs.delete(start_file); - } else { - // Fail closed: keep the start mark so the NEXT launch - // is still guarded. This launch is off Vulkan anyway. - std.log.warn("android: crash guard: could not write {s}; keeping {s}", .{ disabled_file, start_file }); - } + if (writeStamp(primary, disabled_file, cur) and !d.is_primary) _ = d.fs.delete(disabled_file); } }, - .unknown => guarded = true, - .changed => fs.delete(start_file), } - }; + if (d.is_primary and state.owned) continue; + switch (reads[i][1]) { + .absent, .read_error => {}, + .present => |bytes| { + const m: Match = if (Stamp.parse(bytes)) |s| compare(s, current) else .same; + switch (m) { + .same => { + // A start that never became stable, left by a process + // that is gone (not this one: not owned). + guarded = true; + if (current.full()) |cur| { + if (writeStamp(primary, disabled_file, cur)) { + _ = d.fs.delete(start_file); + } else { + // Fail closed: keep the start mark so the + // NEXT launch is still guarded. + std.log.warn("android: crash guard: could not write {s}; keeping {s}", .{ disabled_file, start_file }); + } + } + }, + .unknown => guarded = true, + .changed => _ = d.fs.delete(start_file), + } + }, + } + } return guarded; } @@ -211,7 +273,7 @@ fn writeAtomic(fs: anytype, name: []const u8, bytes: []const u8) bool { var tmp_buf: [64]u8 = undefined; const tmp = std.fmt.bufPrint(&tmp_buf, "{s}.tmp", .{name}) catch return false; if (!fs.write(tmp, bytes) or !fs.rename(tmp, name)) { - fs.delete(tmp); + _ = fs.delete(tmp); return false; } return true; @@ -293,6 +355,19 @@ pub fn beginStart(fs: anytype, current: Stamp, state: *StartState, src: anytype, return true; } +/// A new renderer resolution in this process (whatever it picks): bump the +/// generation so every earlier stable thread stops, WITHOUT touching the +/// start mark. A Vulkan start that was superseded before it reached the +/// threshold stays "incomplete" (the mark, still `owned`, is only cleared by +/// a Vulkan start that itself becomes stable); otherwise a GLES relaunch's +/// frames (after bgfx re-init resets the counter) could clear the Vulkan +/// mark of a start that never presented 120 frames. +pub fn supersede(state: *StartState) void { + state.acquire(); + defer state.release(); + @atomicStore(u32, &state.generation, state.generation +% 1, .release); +} + /// Frames presented since this start: the counter minus `baseline.*`. A /// counter BELOW the baseline means bgfx re-initialised (labelle-bgfx#182 /// resets it on every successful init): the baseline drops to 0 for good. @@ -322,6 +397,7 @@ fn framesSince(raw: u64, baseline: *?u64) u64 { pub fn settle(fs: anytype, clock: anytype, src: anytype, state: *StartState, mine: u32, baseline: ?u64) void { const t0 = clock.now(); var noted = false; + var unlink_failure_logged = false; var base = baseline; // Advisory early exit; the authoritative check is under the lock below. while (state.peek() == mine) { @@ -334,12 +410,18 @@ pub fn settle(fs: anytype, clock: anytype, src: anytype, state: *StartState, min const enough_frames = if (raw) |r| framesSince(r, &base) >= stable_min_frames else true; if (elapsed >= stable_after_ns and enough_frames) { state.acquire(); - defer state.release(); - if (state.generation == mine) { - fs.delete(start_file); + const done = if (state.generation != mine) true else if (fs.delete(start_file)) blk: { state.owned = false; + break :blk true; + } else false; + state.release(); + if (done) return; + // The unlink failed: the mark is still ours (owned stays true), + // so retry on the next poll for as long as the process lives. + if (!unlink_failure_logged) { + unlink_failure_logged = true; + std.log.warn("android: crash guard: could not delete {s}; retrying", .{start_file}); } - return; } clock.sleep(poll_interval_ns) catch return; } @@ -361,6 +443,10 @@ extern "c" fn fwrite(ptr: [*]const u8, size: usize, n: usize, f: *FILE) usize; extern "c" fn fclose(f: *FILE) c_int; extern "c" fn unlink(name: [*:0]const u8) c_int; extern "c" fn rename(from: [*:0]const u8, to: [*:0]const u8) c_int; +extern "c" fn ferror(f: *FILE) c_int; +/// bionic's `errno` location. +extern "c" fn __errno() *c_int; +const enoent: c_int = 2; extern "c" fn usleep(usec: c_uint) c_int; /// Marker files under a copied directory path (a value type, so the @@ -381,12 +467,15 @@ const LibcFs = struct { return std.fmt.bufPrintZ(buf, "{s}/{s}", .{ self.dir_buf[0..self.dir_len], name }) catch null; } - pub fn read(self: *const LibcFs, name: []const u8, buf: []u8) ?[]const u8 { + pub fn read(self: *const LibcFs, name: []const u8, buf: []u8) ReadResult { var p: [600]u8 = undefined; - const f = fopen((self.full(name, &p) orelse return null).ptr, "rb") orelse return null; + const path_z = self.full(name, &p) orelse return .read_error; + const f = fopen(path_z.ptr, "rb") orelse + return if (__errno().* == enoent) .absent else .read_error; defer _ = fclose(f); const n = fread(buf.ptr, 1, buf.len, f); - return buf[0..n]; + if (ferror(f) != 0) return .read_error; + return .{ .present = buf[0..n] }; } pub fn write(self: *const LibcFs, name: []const u8, bytes: []const u8) bool { var p: [600]u8 = undefined; @@ -401,9 +490,10 @@ const LibcFs = struct { const t = self.full(to, &pt) orelse return false; return rename_c(f.ptr, t.ptr) == 0; } - pub fn delete(self: *const LibcFs, name: []const u8) void { + pub fn delete(self: *const LibcFs, name: []const u8) bool { var p: [600]u8 = undefined; - _ = unlink((self.full(name, &p) orelse return).ptr); + const path_z = self.full(name, &p) orelse return false; + return unlink(path_z.ptr) == 0 or __errno().* == enoent; } }; const rename_c = rename; @@ -465,27 +555,49 @@ fn versionCode(activity: *const anyopaque) ?i64 { return v; } -var no_backup_failure_logged: std.atomic.Value(bool) = .init(false); +/// `/../no_backup`: where `getNoBackupFilesDir()` lives next to +/// `internalDataPath` (`/files`). Only used to READ markers a process +/// that did get the JNI answer may have left there. +fn siblingNoBackup(internal: []const u8, buf: []u8) ?[]const u8 { + const trimmed = std.mem.trimEnd(u8, internal, "/"); + const slash = std.mem.lastIndexOfScalar(u8, trimmed, '/') orelse return null; + if (slash == 0) return null; + return std.fmt.bufPrint(buf, "{s}/no_backup", .{trimmed[0..slash]}) catch null; +} + +/// The marker directories, resolved ONCE per process: `primary` is where +/// markers are written, `secondary` the other location, read at launch. +const MarkerDirs = struct { primary: LibcFs, secondary: ?LibcFs }; -/// The markers' directory: `getNoBackupFilesDir()` (never backed up or -/// transferred), else `internalDataPath` with a one-time warning. -fn libcFs(activity: *const anyopaque) ?LibcFs { +var marker_dirs: ?MarkerDirs = null; +var marker_dirs_lock: std.atomic.Mutex = .unlocked; +var marker_dirs_resolved: bool = false; + +/// `getNoBackupFilesDir()` (never backed up or transferred) is primary and +/// `internalDataPath` secondary; if the JNI lookup fails, `internalDataPath` +/// is primary (logged) and the derived `/no_backup` secondary. +fn markerDirs(activity: *const anyopaque) ?MarkerDirs { + while (!marker_dirs_lock.tryLock()) std.atomic.spinLoopHint(); + defer marker_dirs_lock.unlock(); + if (marker_dirs_resolved) return marker_dirs; + marker_dirs_resolved = true; + + const internal: ?LibcFs = if (labelle_android_internal_data_path(activity)) |d| LibcFs.init(std.mem.span(d)) else null; var nb: [512]u8 = undefined; const n = labelle_android_no_backup_dir(activity, &nb, nb.len); - if (n > 0) { - if (LibcFs.init(nb[0..@intCast(n)])) |fs| return fs; - } - if (logOnce(&no_backup_failure_logged)) { - std.log.warn("android: crash guard: could not get noBackupFilesDir; using internalDataPath (Auto Backup may copy the crash-guard markers)", .{}); - } - const dir = labelle_android_internal_data_path(activity) orelse { - std.log.warn("android: crash guard: no internalDataPath; guard off", .{}); - return null; + if (n > 0) if (LibcFs.init(nb[0..@intCast(n)])) |no_backup| { + marker_dirs = .{ .primary = no_backup, .secondary = internal }; + return marker_dirs; }; - return LibcFs.init(std.mem.span(dir)) orelse { - std.log.warn("android: crash guard: internalDataPath unusable; guard off", .{}); + std.log.warn("android: crash guard: could not get noBackupFilesDir; using internalDataPath for this process (Auto Backup may copy the crash-guard markers)", .{}); + const primary = internal orelse { + std.log.warn("android: crash guard: no usable internalDataPath; guard off", .{}); return null; }; + var sib: [512]u8 = undefined; + const secondary: ?LibcFs = if (siblingNoBackup(primary.dir_buf[0..primary.dir_len], &sib)) |d| LibcFs.init(d) else null; + marker_dirs = .{ .primary = primary, .secondary = secondary }; + return marker_dirs; } /// True when a previous launch crashed during a Vulkan start (and the app @@ -498,8 +610,17 @@ fn libcFs(activity: *const anyopaque) ?LibcFs { pub fn vulkanDisabled(activity: ?*const anyopaque, setting: ?[]const u8) bool { if (comptime !is_android) return false; const a = activity orelse return false; - var fs = libcFs(a) orelse return false; - return checkObserved(&fs, .{ .version_code = versionCode(a), .setting = setting }, &start_state); + var dirs = markerDirs(a) orelse return false; + const sec: ?*LibcFs = if (dirs.secondary) |*s2| s2 else null; + return checkDirs(&dirs.primary, sec, .{ .version_code = versionCode(a), .setting = setting }, &start_state); +} + +/// Called by `renderer.resolve` at the start of EVERY renderer resolution: +/// stops earlier stable threads of this process without clearing their +/// start mark (see `supersede`). +pub fn beginResolution() void { + if (comptime !is_android) return; + supersede(&start_state); } /// Called by `renderer.resolve` once Vulkan is chosen, before bgfx starts: @@ -512,7 +633,7 @@ pub fn vulkanDisabled(activity: ?*const anyopaque, setting: ?[]const u8) bool { pub fn beginVulkanStart(activity: ?*const anyopaque, setting: ?[]const u8) bool { if (comptime !is_android) return true; const a = activity orelse return false; - const fs = libcFs(a) orelse return false; + const fs = (markerDirs(a) orelse return false).primary; const obs: Observed = .{ .version_code = versionCode(a), .setting = setting }; const stamp = obs.full() orelse { std.log.warn("android: crash guard: versionCode or {s} unreadable; cannot record this Vulkan start", .{"labelle.renderer"}); @@ -550,6 +671,10 @@ const FakeFs = struct { fail_writes: bool = false, /// Every write stores HALF the bytes, then reports failure (ENOSPC). partial_writes: bool = false, + /// Reads of this file name report a read error (EIO, EMFILE, ...). + read_error_on: ?[]const u8 = null, + /// The next N deletes of an EXISTING file fail (the file stays). + failing_deletes: usize = 0, fn slot(self: *FakeFs, name: []const u8) struct { *?[]const u8, []u8 } { if (std.mem.eql(u8, name, start_file)) return .{ &self.start, &self.bufs[0] }; @@ -558,11 +683,12 @@ const FakeFs = struct { if (std.mem.eql(u8, name, disabled_file ++ ".tmp")) return .{ &self.disabled_tmp, &self.bufs[3] }; @panic("unexpected file name"); } - pub fn read(self: *FakeFs, name: []const u8, buf: []u8) ?[]const u8 { + pub fn read(self: *FakeFs, name: []const u8, buf: []u8) ReadResult { + if (self.read_error_on) |bad| if (std.mem.eql(u8, bad, name)) return .read_error; const s = self.slot(name); - const v = s[0].* orelse return null; + const v = s[0].* orelse return .absent; @memcpy(buf[0..v.len], v); - return buf[0..v.len]; + return .{ .present = buf[0..v.len] }; } pub fn write(self: *FakeFs, name: []const u8, bytes: []const u8) bool { if (self.fail_writes) return false; @@ -581,8 +707,14 @@ const FakeFs = struct { f[0].* = null; return true; } - pub fn delete(self: *FakeFs, name: []const u8) void { - self.slot(name)[0].* = null; + pub fn delete(self: *FakeFs, name: []const u8) bool { + const s = self.slot(name); + if (s[0].* != null and self.failing_deletes > 0) { + self.failing_deletes -= 1; + return false; + } + s[0].* = null; + return true; } fn set(self: *FakeFs, name: []const u8, bytes: []const u8) void { const s = self.slot(name); @@ -1292,3 +1424,167 @@ test "a start mark that cannot be written falls back to gles (unless the intent try testing.expectEqual(R.Renderer.vulkan, ok.renderer); try testing.expectEqual(R.Source.setting, ok.source); } + +// ── Codex VERDICT at 7c28035 (labelle-bgfx#172 comments 5909398772 / 5909522964) ── + +test "GLES same-process relaunch: the old Vulkan timer can no longer clear the mark" { + // Vulkan start (gen N, mark written) → the Activity is relaunched in the + // same process and resolves to GLES → GLES init resets the counter and + // its frames pass 120. The gen-N thread must NOT clear the Vulkan mark. + var fs: FakeFs = .{}; + var state: StartState = .{}; + supersede(&state); // resolution 1 + var got: ?u32 = null; + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, RecordingSpawner{ .got = &got })); + const vulkan_gen = got.?; + + // Resolution 2 (GLES): only supersede, no beginStart. + var clock: FakeClock = .{}; + const Relaunch = struct { + inner: FakeFrames, + state: *StartState, + pub fn framesPresented(self: *@This()) ?u64 { + // The GLES relaunch lands on the 2nd poll (0.5 s). + if (self.inner.calls == 1) supersede(self.state); + return self.inner.framesPresented(); + } + pub fn noteTimeOnly(self: *@This()) void { + self.inner.noteTimeOnly(); + } + }; + // GLES init at 1 s resets the counter, then 60 fps (540 frames by 10 s). + var frames: Relaunch = .{ .inner = .{ .clock = &clock, .init_done_ns = std.time.ns_per_s }, .state = &state }; + settle(&fs, &clock, &frames, &state, vulkan_gen, 0); + try testing.expect(clock.now_ns < stable_after_ns); // it stopped early... + try testing.expectEqualStrings("1 vulkan", fs.start.?); // ...and kept the mark + try testing.expect(state.owned); // still this process's incomplete start + + // The process later exits normally without another Vulkan start: the + // next process treats the Vulkan start as incomplete. + try testing.expect(checkAtLaunch(&fs, v1_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); +} + +test "a later Vulkan start in the same process can still clear the mark itself" { + var fs: FakeFs = .{}; + var state: StartState = .{}; + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, NoSpawner{})); + supersede(&state); // a GLES relaunch + // Vulkan again (same process): its own mark is not a crash... + try testing.expect(!checkObserved(&fs, Observed.of(v1_vulkan), &state)); + var got: ?u32 = null; + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, RecordingSpawner{ .got = &got })); + // ...and once THIS start is stable, it clears the mark. + var clock: FakeClock = .{}; + var frames: FakeFrames = .{ .clock = &clock }; + settle(&fs, &clock, &frames, &state, got.?, null); + try testing.expect(fs.start == null); + try testing.expect(!state.owned); +} + +test "a marker read error fails closed: guarded, nothing written or deleted" { + // Disabled marker unreadable (EIO): guarded even though a start-free, + // version-bumped launch would otherwise reset. + var fs: FakeFs = .{}; + fs.set(disabled_file, "1 vulkan"); + fs.read_error_on = disabled_file; + try testing.expect(checkAtLaunch(&fs, v2_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs.disabled.?); // not deleted + + // Start marker unreadable, with a stale disabled marker that would reset. + var fs2: FakeFs = .{}; + fs2.set(disabled_file, "1 vulkan"); + fs2.set(start_file, "2 vulkan"); + fs2.read_error_on = start_file; + try testing.expect(checkAtLaunch(&fs2, v2_vulkan)); + try testing.expectEqualStrings("1 vulkan", fs2.disabled.?); // untouched + try testing.expectEqualStrings("2 vulkan", fs2.start.?); + + // Readable again: the normal rules apply (the v2 start is a crash). + fs2.read_error_on = null; + try testing.expect(checkAtLaunch(&fs2, v2_vulkan)); + try testing.expectEqualStrings("2 vulkan", fs2.disabled.?); +} + +test "a failed unlink keeps ownership and retries; stable start is not left 'incomplete'" { + var fs: FakeFs = .{}; + var state: StartState = .{}; + var got: ?u32 = null; + try testing.expect(beginStart(&fs, v1_vulkan, &state, NoFrames{}, RecordingSpawner{ .got = &got })); + fs.failing_deletes = 3; // three transient unlink failures + var clock: FakeClock = .{}; + var frames: FakeFrames = .{ .clock = &clock }; + settle(&fs, &clock, &frames, &state, got.?, null); + try testing.expect(fs.start == null); // cleared on the 4th attempt + try testing.expect(!state.owned); + try testing.expectEqual(stable_after_ns + 3 * poll_interval_ns, clock.now_ns); + + // While it keeps failing, ownership is kept, so a same-process relaunch + // does not read this stable start as a crash. + var fs2: FakeFs = .{}; + var state2: StartState = .{}; + got = null; + try testing.expect(beginStart(&fs2, v1_vulkan, &state2, NoFrames{}, RecordingSpawner{ .got = &got })); + fs2.failing_deletes = std.math.maxInt(usize); + var clock2: FakeClock = .{ .dies_at_ns = 20 * std.time.ns_per_s }; + var frames2: FakeFrames = .{ .clock = &clock2 }; + settle(&fs2, &clock2, &frames2, &state2, got.?, null); + try testing.expect(state2.owned); + try testing.expect(fs2.start != null); + fs2.failing_deletes = 0; + try testing.expect(!checkObserved(&fs2, Observed.of(v1_vulkan), &state2)); + try testing.expect(fs2.disabled == null); +} + +test "both marker dirs are checked; writes go only to the primary" { + // A disabled marker in the OTHER dir (older build / fallback launch). + var primary: FakeFs = .{}; + var other: FakeFs = .{}; + other.set(disabled_file, "1 vulkan"); + var st: StartState = .{}; + try testing.expect(checkDirs(&primary, &other, Observed.of(v1_vulkan), &st)); + try testing.expect(primary.disabled == null); // nothing copied/written + try testing.expectEqualStrings("1 vulkan", other.disabled.?); + + // A crashed start in the other dir: the disabled marker goes to the + // primary, the start mark is removed from the other dir. + var p2: FakeFs = .{}; + var o2: FakeFs = .{}; + o2.set(start_file, "1 vulkan"); + var st2: StartState = .{}; + try testing.expect(checkDirs(&p2, &o2, Observed.of(v1_vulkan), &st2)); + try testing.expectEqualStrings("1 vulkan", p2.disabled.?); + try testing.expect(o2.start == null and o2.disabled == null); + + // A read error in the other dir fails closed too. + var p3: FakeFs = .{}; + var o3: FakeFs = .{ .read_error_on = start_file }; + var st3: StartState = .{}; + try testing.expect(checkDirs(&p3, &o3, Observed.of(v1_vulkan), &st3)); + + // A stale marker in the other dir is reset there; nothing else happens. + var p4: FakeFs = .{}; + var o4: FakeFs = .{}; + o4.set(disabled_file, "1 vulkan"); + var st4: StartState = .{}; + try testing.expect(!checkDirs(&p4, &o4, Observed.of(v2_vulkan), &st4)); + try testing.expect(o4.disabled == null); + + // The primary's OWN live mark is skipped (same process), but a start + // mark in the other dir is never "owned". + var p5: FakeFs = .{}; + var o5: FakeFs = .{}; + var st5: StartState = .{}; + try testing.expect(beginStart(&p5, v1_vulkan, &st5, NoFrames{}, NoSpawner{})); + try testing.expect(!checkDirs(&p5, &o5, Observed.of(v1_vulkan), &st5)); + o5.set(start_file, "1 vulkan"); + try testing.expect(checkDirs(&p5, &o5, Observed.of(v1_vulkan), &st5)); +} + +test "siblingNoBackup derives /no_backup from internalDataPath" { + var buf: [128]u8 = undefined; + try testing.expectEqualStrings("/data/user/0/com.x/no_backup", siblingNoBackup("/data/user/0/com.x/files", &buf).?); + try testing.expectEqualStrings("/data/user/0/com.x/no_backup", siblingNoBackup("/data/user/0/com.x/files/", &buf).?); + try testing.expect(siblingNoBackup("/files", &buf) == null); + try testing.expect(siblingNoBackup("files", &buf) == null); +} diff --git a/src/renderer.zig b/src/renderer.zig index ea3d681..8fd9836 100644 --- a/src/renderer.zig +++ b/src/renderer.zig @@ -273,6 +273,9 @@ const JniQuery = struct { pub fn resolve(activity: ?*const anyopaque, intent_extra: ?[:0]const u8) void { if (comptime !is_android) return; const a = activity orelse return; + // Every resolution supersedes this process's earlier Vulkan stable + // thread (whatever it picks), keeping its start mark "incomplete". + crash_guard.beginResolution(); var q: JniQuery = .{ .activity = a, .extra = intent_extra }; var d = decide(&q); if (d.renderer == .vulkan) d = afterStart(d, crash_guard.beginVulkanStart(a, q.metaRead().guardSetting()));