Skip to content

Add 5s hang watchdog and one-shot Android FrameSync→VSync migration - #245

Merged
winnerspiros merged 2 commits into
masterfrom
copilot/fix-black-screen-issue
Apr 22, 2026
Merged

Add 5s hang watchdog and one-shot Android FrameSync→VSync migration#245
winnerspiros merged 2 commits into
masterfrom
copilot/fix-black-screen-issue

Conversation

Copilot AI commented Apr 22, 2026

Copy link
Copy Markdown

Android freeze on S23 Ultra (Adreno 740) with no native crash and no actionable signal in runtime.log — the process stops drawing but never dies, so existing crash diagnostics never fire.

HangWatchdog (osu.Android/HangWatchdog.cs)

  • Per-GameThread heartbeat (Update/Draw/Audio/Input) refreshed every 1 s via Scheduler.AddDelayed(repeat:true). The delegate runs on the game thread itself, so its execution is the liveness signal.
  • Dedicated background thread polls every 1 s; on >5 s heartbeat staleness (or >5 s armed-but-never-ticked) dumps for every /proc/self/task/<tid>: comm, wchan, syscall, parsed state code from stat, plus all four heartbeat ages and Linux tids.
  • wchan/syscall is the actionable bit: names the kernel function each thread is parked in (vkAcquireNextImageKHR futex, Realm fifo read, AAudio poll, GC sweep, …) without needing adb.
  • Per-thread re-dump cooldown 10 s, hard cap 200 dumps/process — fits comfortably under the 70 MB log budget even in a permanent hang. Auto re-arms after each dump so recovery is visible.
  • Writes through a new public CrashDiagnostics.AppendDiagnosticBlock(string) so dumps land in the same internal+external native_crash.log the rest of the diagnostics use.

One-shot FrameSyncVSync migration on Android

  • New OsuSetting.AndroidStartupFrameSyncMigrationApplied, applied once in OsuGameAndroid.load() and only when the current value still equals the framework default Limit2x.
  • On a 120 Hz Adreno display, Limit2x targets ~240 fps. With 2–3 swapchain images the draw thread can queue presents faster than the GPU drains, so vkAcquireNextImageKHR stalls on the present-queue futex. Combined with bursty texture uploads from the load thread, this starves the draw thread for seconds — matches the freeze profile. VSync bounds in-flight frames to 1.
  • Idempotent: user choices made later in Settings → Graphics → Renderer are not overridden on subsequent launches.
// OsuGameAndroid.load()
private void applyAndroidFrameSyncMigrationOnce(FrameworkConfigManager frameworkConfig)
{
    if (LocalConfig.Get<bool>(OsuSetting.AndroidStartupFrameSyncMigrationApplied))
        return;

    var frameSync = frameworkConfig.GetBindable<FrameSync>(FrameworkSetting.FrameSync);
    if (frameSync.Value == FrameSync.Limit2x)
        frameSync.Value = FrameSync.VSync;

    LocalConfig.SetValue(OsuSetting.AndroidStartupFrameSyncMigrationApplied, true);
}

Out of scope

No framework/Veldrid changes, no changes to existing crash handler, Vulkan probe, or Oboe bridge. Watchdog overhead is four Interlocked.Exchanges per second on the game threads.

Copilot AI and others added 2 commits April 22, 2026 22:02
@winnerspiros
winnerspiros marked this pull request as ready for review April 22, 2026 22:05
Copilot AI review requested due to automatic review settings April 22, 2026 22:05
@winnerspiros
winnerspiros merged commit 91c259c into master Apr 22, 2026
5 of 15 checks passed
@gitar-bot

gitar-bot Bot commented Apr 22, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds Android-side diagnostics and a one-time configuration migration to address device-specific “stops drawing but doesn’t crash” hangs by (1) detecting multi-second stalls and dumping /proc/self/task state into native_crash.log, and (2) migrating the Android default FrameSync away from an overly aggressive default that can exacerbate Vulkan present-queue stalls.

Changes:

  • Introduces HangWatchdog to monitor Update/Draw/Audio/Input thread liveness and dump /proc thread snapshots on >5s stalls.
  • Adds a one-shot Android FrameSync.Limit2x → FrameSync.VSync migration gated by a new OsuSetting.AndroidStartupFrameSyncMigrationApplied flag.
  • Exposes CrashDiagnostics.AppendDiagnosticBlock(string) as a shared sink for non-crash diagnostic dumps into native_crash.log.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
osu.Game/Configuration/OsuConfigManager.cs Adds the new persisted setting flag and default value for the one-shot migration gate.
osu.Android/OsuGameAndroid.cs Injects FrameworkConfigManager, applies the one-shot FrameSync migration during load, and starts the hang watchdog after SetHost().
osu.Android/HangWatchdog.cs New watchdog implementation that schedules per-thread heartbeats and writes /proc/self/task snapshots on detected stalls.
osu.Android/CrashDiagnostics.cs Adds AppendDiagnosticBlock() to append watchdog (and other) diagnostic blocks to the existing crash logs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +231 to +232
collected.Sort(StringComparer.Ordinal);

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says this is “Sort by tid”, but collected.Sort(StringComparer.Ordinal) sorts lexicographically, not numerically (e.g. "100" comes before "20"). If you want stable ordering by numeric tid, parse to int and sort numerically (falling back when parsing fails).

Suggested change
collected.Sort(StringComparer.Ordinal);
collected.Sort((x, y) =>
{
bool xParsed = int.TryParse(x, out int xTid);
bool yParsed = int.TryParse(y, out int yTid);
if (xParsed && yParsed)
return xTid.CompareTo(yTid);
if (xParsed)
return -1;
if (yParsed)
return 1;
return StringComparer.Ordinal.Compare(x, y);
});

Copilot uses AI. Check for mistakes.
catch (Exception e)
{
// Diagnostic-only: failing to migrate must never block startup.
Debug.WriteLine($"[osu!] applyAndroidFrameSyncMigrationOnce failed: {e.Message}");

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch block logs only e.Message, which drops stack trace and inner exception context that would be useful when diagnosing why the migration failed. Consider logging e (e.g. e.ToString()) and/or routing it through the existing CrashDiagnostics/Logger mechanisms used elsewhere on Android.

Suggested change
Debug.WriteLine($"[osu!] applyAndroidFrameSyncMigrationOnce failed: {e.Message}");
Debug.WriteLine($"[osu!] applyAndroidFrameSyncMigrationOnce failed: {e}");

Copilot uses AI. Check for mistakes.
Comment on lines +883 to +885
// GameThread instances. Running on a dedicated background thread, it
// ticks each thread's Scheduler every ~1s and dumps a /proc/self/task
// snapshot if any thread fails to drain its queue for >5s.

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment says the watchdog “ticks each thread's Scheduler every ~1s”, but the implementation schedules a recurring no-op on each GameThread and then monitors the heartbeat timestamps from a background thread. Consider rewording to avoid implying the watchdog actively drives/ticks schedulers from the monitor thread.

Suggested change
// GameThread instances. Running on a dedicated background thread, it
// ticks each thread's Scheduler every ~1s and dumps a /proc/self/task
// snapshot if any thread fails to drain its queue for >5s.
// GameThread instances. It schedules a recurring heartbeat on each
// GameThread and, from a dedicated background monitor thread, watches
// those heartbeat timestamps and dumps a /proc/self/task snapshot if
// any thread stops draining its queue for >5s.

Copilot uses AI. Check for mistakes.
Comment on lines +150 to +153
// Re-arm so that if the thread eventually recovers we
// start counting from the recovery point, not the start
// of the original hang.
hb.Arm();

Copilot AI Apr 22, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hb.Arm() is called after every hang dump, but Heartbeat.Arm() schedules a repeating Scheduler.AddDelayed(..., repeat: true) each time. This will accumulate multiple repeating heartbeats per thread after repeated hangs/re-dumps (extra queued work every second, and potential memory/leakage). Consider scheduling the repeating tick exactly once per thread (store/cancel the scheduled delegate), and on re-arm only reset the timestamps/counters instead of adding another repeating schedule.

Suggested change
// Re-arm so that if the thread eventually recovers we
// start counting from the recovery point, not the start
// of the original hang.
hb.Arm();
// Reset the watchdog reference point so that if the
// thread eventually recovers we start counting from
// the recovery point, not the start of the original
// hang. Avoid calling hb.Arm() here because Arm()
// schedules a repeating heartbeat; re-calling it after
// every dump would accumulate duplicate repeating
// scheduled callbacks for the same thread.
Interlocked.Exchange(ref hb.ArmedAtUtcMs, nowMs);
Interlocked.Exchange(ref hb.LastTickUtcMs, nowMs);

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants