Merge pull request #262 from winnerspiros/copilot/fix-black-screen-issue-acb0fbe0-a5b9-416c-857c-dd090feb0404

Fix CS1513 in LogManagement and double-dispose race on cold-start timers

Author: winnerspiros

osu.Android/LogManagement.cs

@@ -238,6 +238,161 @@ public static void NormaliseFrameworkIniExecutionMode()
             }
         }
 
+        // Sentinel file dropped after the one-shot Renderer-default migration has
+        // run. Stored in the storage root next to framework.ini so a single
+        // existence check governs whether we should respect the user's currently
+        // persisted Renderer choice (sentinel present) or perform the one-time
+        // Automatic→OpenGL nudge (sentinel absent).
+        private const string renderer_migration_sentinel = "android_renderer_default_migrated.flag";
+
+        /// <summary>
+        /// One-shot migration that flips the framework default <c>Renderer</c>
+        /// choice from <c>Automatic</c> (which resolves to Vulkan on Android,
+        /// requiring runtime SPIR-V compilation via glslang) to <c>OpenGL</c>
+        /// (which uses the Adreno driver's native GLSL compiler — no glslang,
+        /// no SPIR-V, and therefore no shader-compile burst on Toolbar load).
+        ///
+        /// <para>
+        /// Why this matters: every recent black-screen ANR fingerprint in the
+        /// field tombstones (PIDs 27798 / 29226 / 499) shows a Veldrid worker
+        /// stuck inside <c>glslang::TParseContext::executeInitializer</c> /
+        /// <c>TShader::parse</c> at <c>nice=-10</c> on a big core, monopolising
+        /// the CPU during the Toolbar texture-upload burst and starving the
+        /// Update thread past the 10-second MotionEvent ANR deadline. Switching
+        /// the default away from the Vulkan-via-glslang path eliminates the
+        /// entire failure class on stock installs. Users who specifically want
+        /// Vulkan can still select it from Settings → Graphics → Renderer; the
+        /// migration only nudges the *default* and is recorded by an on-disk
+        /// sentinel so subsequent launches never overwrite an explicit choice.
+        /// </para>
+        ///
+        /// <para>
+        /// Best-effort and never throws — if the file is missing or the rewrite
+        /// fails, startup proceeds with the existing value. Must be invoked from
+        /// <c>OsuGameActivity.OnCreate</c> BEFORE the framework reads
+        /// framework.ini, alongside the existing
+        /// <see cref="NormaliseFrameworkIniExecutionMode"/> hook.
+        /// </para>
+        /// </summary>
+        public static void NormaliseFrameworkIniRendererDefault()
+        {
+            try
+            {
+                string? root = resolveStorageRoot();
+                if (root == null) return;
+
+                string sentinelPath = Path.Combine(root, renderer_migration_sentinel);
+                if (File.Exists(sentinelPath)) return;
+
+                string iniPath = Path.Combine(root, "framework.ini");
+
+                if (!File.Exists(iniPath))
+                {
+                    // Brand-new install: no framework.ini yet. Pre-create a minimal
+                    // file with just the Renderer line set; the framework will fill
+                    // in its other defaults on first save.
+                    try
+                    {
+                        File.WriteAllText(iniPath, "Renderer = OpenGL" + System.Environment.NewLine);
+                        tryDropSentinel(sentinelPath);
+                    }
+                    catch (Exception e)
+                    {
+                        Debug.WriteLine($"[osu!] LogManagement: could not pre-create framework.ini: {e.Message}");
+                    }
+                    return;
+                }
+
+                string[] lines;
+
+                try
+                {
+                    lines = File.ReadAllLines(iniPath);
+                }
+                catch (Exception e)
+                {
+                    Debug.WriteLine($"[osu!] LogManagement: could not read framework.ini for renderer migration: {e.Message}");
+                    return;
+                }
+
+                bool changed = false;
+                bool seenRendererLine = false;
+
+                for (int i = 0; i < lines.Length; i++)
+                {
+                    string line = lines[i];
+                    int eq = line.IndexOf('=');
+                    if (eq <= 0) continue;
+
+                    string key = line.Substring(0, eq).Trim();
+                    string value = line.Substring(eq + 1).Trim();
+
+                    if (!string.Equals(key, "Renderer", StringComparison.Ordinal))
+                        continue;
+
+                    seenRendererLine = true;
+
+                    // Only nudge the default. If the user has explicitly chosen
+                    // Vulkan / OpenGLLegacy / Direct3D11 / Metal / Deferred, leave
+                    // it alone — the migration's job is to change the *default*,
+                    // not overwrite intent.
+                    if (string.Equals(value, "Automatic", StringComparison.Ordinal))
+                    {
+                        lines[i] = "Renderer = OpenGL";
+                        changed = true;
+                    }
+
+                    break;
+                }
+
+                if (!seenRendererLine)
+                {
+                    // No Renderer line at all — append one at the end of the file.
+                    var newLines = new string[lines.Length + 1];
+                    Array.Copy(lines, newLines, lines.Length);
+                    newLines[lines.Length] = "Renderer = OpenGL";
+                    lines = newLines;
+                    changed = true;
+                }
+
+                if (changed)
+                {
+                    try
+                    {
+                        File.WriteAllLines(iniPath, lines);
+                        Logger.Log("[osu!] Android first-launch Renderer-default migration: Automatic → OpenGL", LoggingTarget.Performance);
+                    }
+                    catch (Exception e)
+                    {
+                        Debug.WriteLine($"[osu!] LogManagement: could not rewrite framework.ini for renderer migration: {e.Message}");
+                        return;
+                    }
+                }
+
+                // Drop sentinel regardless of whether we changed anything: the
+                // migration has now had its one chance to run, and any subsequent
+                // user choice (including a deliberate "Automatic") must be
+                // respected.
+                tryDropSentinel(sentinelPath);
+            }
+            catch (Exception e)
+            {
+                Debug.WriteLine($"[osu!] LogManagement: NormaliseFrameworkIniRendererDefault failed: {e.Message}");
+            }
+        }
+
+        private static void tryDropSentinel(string sentinelPath)
+        {
+            try
+            {
+                File.WriteAllText(sentinelPath, string.Empty);
+            }
+            catch (Exception e)
+            {
+                Debug.WriteLine($"[osu!] LogManagement: could not write renderer-migration sentinel: {e.Message}");
+            }
+        }
+
         private static string? resolveStorageRoot()
         {
             try

osu.Android/OsuGameActivity.cs

@@ -107,6 +107,20 @@ protected override void OnCreate(Bundle? savedInstanceState)
             // OnCreate so any crash from this point onward lands in `native_crash.log`.
             CrashDiagnostics.InstallNativeHandler(this);
             CrashDiagnostics.InstallManagedExceptionHooks();
+
+            // Mirror the PREVIOUS session's internal native_crash.log into the external
+            // copy, then truncate the internal file BEFORE we write any markers for the
+            // current session. Doing this earlier (it used to run after the first three
+            // WriteAliveMarker / WriteInstallState calls) caused those three early lines
+            // to appear duplicated on disk: they were written directly to both
+            // internal+external, then the mirror appended the internal copy onto
+            // external, doubling them. Field native_crash.log files confirm this
+            // (Activity.OnCreate entry / INSTALL_STATE / StartNativeWatchdog all appear
+            // twice with identical timestamps, the rest of the file singly). Running
+            // the mirror first folds in last session's content cleanly and lets all
+            // current-session markers land exactly once in each file.
+            CrashDiagnostics.MirrorInternalLogToExternal();
+
             CrashDiagnostics.WriteAliveMarker("Activity.OnCreate entry");
             CrashDiagnostics.WriteInstallState();
             // Arm the native pthread liveness watchdog as the very next thing,
@@ -119,7 +133,6 @@ protected override void OnCreate(Bundle? savedInstanceState)
             // monitor) is parked in __rt_sigsuspend during a stuck GC. 10s
             // threshold matches the Android system-server's own ANR window.
             CrashDiagnostics.StartNativeWatchdog(10);
-            CrashDiagnostics.MirrorInternalLogToExternal();
 
             // Crash-loop safe-mode latch. If the previous process died (ANR / native
             // crash / OOM kill) before reaching the post-LoadComplete clear point,
@@ -176,6 +189,16 @@ protected override void OnCreate(Bundle? savedInstanceState)
             LogManagement.NormaliseFrameworkIniExecutionMode();
             CrashDiagnostics.WriteAliveMarker("LogManagement.NormaliseFrameworkIniExecutionMode (returned)");
 
+            // One-shot Renderer-default migration: Automatic → OpenGL on Android.
+            // Eliminates the Veldrid glslang/SPIR-V shader-compile burst that has
+            // been the proximate cause of the recurring Toolbar-time MotionEvent
+            // ANR on Adreno devices. User can still pick Vulkan from
+            // Settings → Graphics → Renderer; the migration only nudges the
+            // default and never re-runs (governed by an on-disk sentinel).
+            CrashDiagnostics.WriteAliveMarker("LogManagement.NormaliseFrameworkIniRendererDefault (about to start)");
+            LogManagement.NormaliseFrameworkIniRendererDefault();
+            CrashDiagnostics.WriteAliveMarker("LogManagement.NormaliseFrameworkIniRendererDefault (returned)");
+
             CrashDiagnostics.WriteAliveMarker("LogManagement.WipeShaderCacheOnceForVersion (about to start)");
             LogManagement.WipeShaderCacheOnceForVersion();
             CrashDiagnostics.WriteAliveMarker("LogManagement.WipeShaderCacheOnceForVersion (returned)");

osu.Android/OsuGameAndroid.cs

@@ -95,6 +95,16 @@ public partial class OsuGameAndroid : OsuGame
         private Delegate? activeMixersHandler;
         private object? activeMixersList;
 
+        // Cold-start safety nets that MUST keep firing even if the Update thread
+        // stalls on a Veldrid glslang shader-compile burst. Held as fields so the
+        // .NET threadpool kernel timer keeps the underlying ManagedTimerHolder
+        // alive (a System.Threading.Timer with no live root is eligible for GC).
+        // See LoadComplete for the rationale (Scheduler.AddDelayed runs on the
+        // Update thread and therefore cannot be relied on to fire the very
+        // safety nets that exist to unblock that thread).
+        private System.Threading.Timer? coldStartTamingTimer;
+        private System.Threading.Timer? clearStartupSentinelTimer;
+
         // Set true by the deferred SelectHighestRefreshRate call in LoadComplete; gates
         // any earlier OnConfigurationChanged-driven SelectHighestRefreshRate() invocations
         // out of the cold-start swapchain bring-up window. See SelectHighestRefreshRate.
@@ -495,9 +505,19 @@ protected override void LoadComplete()
             // on the first CompileGlslToSpirv call, which happens mid-Toolbar-load
             // (i.e. after the synchronous taming pass above has already run). Without
             // these follow-up passes, the newly-spawned worker inherits nice=-10
-            // from its parent and reproduces the starvation pattern. The chosen
-            // timestamps straddle the observed "Texture upload queue is large (100/
-            // 200/300)" events in the field runtime logs.
+            // from its parent and reproduces the starvation pattern.
+            //
+            // CRITICAL: these passes MUST run on the .NET threadpool (System.Threading.Timer),
+            // NOT on Scheduler.AddDelayed. Scheduler runs on the Update thread, which is
+            // exactly what we're trying to unblock — if the glslang worker has already
+            // started monopolising a big core at nice=-10 by the time the first deferred
+            // Scheduler tick is due, the Update thread is already starved and the tick
+            // never fires. Field tombstones (PIDs 27798/29226/499) confirm this: the +0
+            // and +500ms taming passes logged, but +1500/+3500ms never did, while a
+            // glslang worker remained at nice=-10 producing the 10s MotionEvent ANR.
+            // A kernel-managed Timer fires from the threadpool regardless of game-thread
+            // health, so the just-spawned worker is reliably caught and demoted within
+            // one tick (250 ms) of being created.
             try
             {
                 int coreCount = System.Environment.ProcessorCount;
@@ -513,27 +533,37 @@ protected override void LoadComplete()
                     if (deferredLittleMask == 0) deferredLittleMask = totalMask;
                 }
 
-                foreach (int delayMs in new[] { 500, 1500, 3500 })
+                int capturedMask = deferredLittleMask;
+                int tickCount = 0;
+                // Tick every 250 ms, give up after ~8 s — long enough to cover the entire
+                // observed Toolbar shader-compile burst window (mid-load through drain).
+                const int tick_period_ms = 250;
+                const int max_ticks = 32;
+
+                coldStartTamingTimer = new System.Threading.Timer(_ =>
                 {
-                    int dm = delayMs;
-                    Scheduler.AddDelayed(() =>
+                    try
                     {
-                        try
-                        {
-                            int demoted = AndroidNativeBridgeManager.TameBackgroundThreads(deferredLittleMask);
-                            if (demoted > 0)
-                                Logger.Log($"[osu!] Tamed {demoted} background worker thread(s) at +{dm}ms", LoggingTarget.Performance);
-                        }
-                        catch (Exception e)
-                        {
-                            Debug.WriteLine($"[osu!] Deferred TameBackgroundThreads(+{dm}ms) failed: {e.Message}");
-                        }
-                    }, delayMs);
-                }
+                        int demoted = AndroidNativeBridgeManager.TameBackgroundThreads(capturedMask);
+                        if (demoted > 0)
+                            Logger.Log($"[osu!] Tamed {demoted} background worker thread(s) (timer tick {tickCount + 1})", LoggingTarget.Performance);
+                    }
+                    catch (Exception e)
+                    {
+                        Debug.WriteLine($"[osu!] Deferred TameBackgroundThreads (timer) failed: {e.Message}");
+                    }
+
+                    if (System.Threading.Interlocked.Increment(ref tickCount) >= max_ticks)
+                    {
+                        var t = System.Threading.Interlocked.Exchange(ref coldStartTamingTimer, null);
+                        try { t?.Dispose(); }
+                        catch { /* ignore */ }
+                    }
+                }, state: null, dueTime: tick_period_ms, period: tick_period_ms);
             }
             catch (Exception e)
             {
-                Debug.WriteLine($"[osu!] Failed to schedule deferred TameBackgroundThreads passes: {e.Message}");
+                Debug.WriteLine($"[osu!] Failed to schedule deferred TameBackgroundThreads timer: {e.Message}");
             }
 
             Scheduler.AddDelayed(() =>
@@ -634,7 +664,36 @@ protected override void LoadComplete()
             // before the user could reasonably trigger a manual restart. If the
             // process dies before this fires (ANR, native crash, OOM kill), the
             // sentinel persists and the next launch enters safe-mode.
-            Scheduler.AddDelayed(AndroidStartupSafeMode.ClearStartupInProgress, 10_000);
+            //
+            // Fired from a kernel-managed System.Threading.Timer rather than
+            // Scheduler.AddDelayed: the same Update-thread stall that caused the
+            // Toolbar shader-compile ANR also prevents Scheduler.AddDelayed from
+            // firing the sentinel-clear, leaving safe-mode latched forever and
+            // every relaunch hitting the identical wall (confirmed by all three
+            // field tombstones — 27798 / 29226 / 499 — starting with "CPU affinity
+            // pinning skipped (safe-mode active)"). The threadpool tick is immune
+            // to game-thread starvation, so the sentinel reliably clears whenever
+            // the activity-main thread (and therefore the process) survives the
+            // deadline, breaking the perpetual-safe-mode loop.
+            try
+            {
+                clearStartupSentinelTimer = new System.Threading.Timer(_ =>
+                {
+                    try { AndroidStartupSafeMode.ClearStartupInProgress(); }
+                    catch (Exception e)
+                    {
+                        Debug.WriteLine($"[osu!] ClearStartupInProgress (timer) failed: {e.Message}");
+                    }
+
+                    var ct = System.Threading.Interlocked.Exchange(ref clearStartupSentinelTimer, null);
+                    try { ct?.Dispose(); }
+                    catch { /* ignore */ }
+                }, state: null, dueTime: 10_000, period: System.Threading.Timeout.Infinite);
+            }
+            catch (Exception e)
+            {
+                Debug.WriteLine($"[osu!] Failed to schedule ClearStartupInProgress timer: {e.Message}");
+            }
 
             // Cold-start heartbeat instrumentation. For the first 15 s after LoadComplete
             // we emit per-second ALIVE markers from BOTH the Update thread and the Draw
@@ -1465,6 +1524,14 @@ protected override void Dispose(bool isDisposing)
                 highPerformanceSession = null;
                 dexPerformanceSession?.Dispose();
                 dexPerformanceSession = null;
+
+                var cst = System.Threading.Interlocked.Exchange(ref coldStartTamingTimer, null);
+                try { cst?.Dispose(); }
+                catch { /* ignore */ }
+
+                var sst = System.Threading.Interlocked.Exchange(ref clearStartupSentinelTimer, null);
+                try { sst?.Dispose(); }
+                catch { /* ignore */ }
             }
         }