Skip to content

Commit fb435de

Browse files
author
Ubuntu
committed
fix: set surface event AFTER SetFormat to preserve draw thread invariant
Review feedback: setting surfaceEvent before SetFormat is dangerous because the draw thread can get a stale surface pointer that's being torn down. New approach: Set surfaceEvent AFTER SetFormat returns. By this point, the synchronous SurfaceDestroyed→SurfaceCreated→SurfaceChanged cycle has already completed and surfaceGlobalRef points to the new RGBA8888 surface. This preserves the invariant that surfaceEvent is only set when surfaceGlobalRef points to a safe-to-render surface. The NativeWatchdog.Heartbeat() is still called before SetFormat to reset the watchdog timer during the blocking teardown.
1 parent 2dd8eeb commit fb435de

1 file changed

Lines changed: 8 additions & 8 deletions

File tree

osu.Android/OsuGameActivity.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -900,14 +900,6 @@ public void SurfaceChanged(ISurfaceHolder holder, global::Android.Graphics.Forma
900900
Logger.Log(rgb565Message, LoggingTarget.Runtime, LogLevel.Important);
901901
Logger.Log(rgb565Message, LoggingTarget.Performance, LogLevel.Important);
902902

903-
// Set the surface event BEFORE anything else so the draw thread can
904-
// proceed with the current (soon-to-be-recreated) surface. Without this,
905-
// the draw thread blocks on surfaceEvent.Wait(5000) and the native watchdog
906-
// fires because no managed heartbeat arrives during the synchronous teardown.
907-
// A brief frame or two with the old surface format is preferable to a
908-
// deadlocked draw thread and watchdog kill.
909-
surfaceEvent.Set();
910-
911903
// Tick the native watchdog BEFORE calling SetFormat. SetFormat triggers a
912904
// synchronous surface teardown on the UI thread that can block for hundreds
913905
// of milliseconds. During this window the Update thread may not get a chance
@@ -935,6 +927,14 @@ public void SurfaceChanged(ISurfaceHolder holder, global::Android.Graphics.Forma
935927
Debug.WriteLine($"[osu!] Failed to request RGBA8888 format change for Vulkan: {e.Message}");
936928
}
937929

930+
// After SetFormat returns, the synchronous SurfaceDestroyed→SurfaceCreated→SurfaceChanged
931+
// cycle has already completed and surfaceGlobalRef points to the new RGBA8888 surface.
932+
// Set the event now so GetSurfaceGlobalRef() returns the new valid surface.
933+
// This preserves the invariant that surfaceEvent is only set when surfaceGlobalRef
934+
// points to a safe-to-render surface, while the watchdog heartbeat above prevents
935+
// a false positive during the blocking SetFormat call.
936+
surfaceEvent.Set();
937+
938938
return;
939939
}
940940

0 commit comments

Comments
 (0)