Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions osu.Android/CrashDiagnostics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,25 @@ internal static class CrashDiagnostics
private static int initialised;
private static int managedHooksInstalled;

// Global cap on FirstChanceException dumps written per process. A hot-path
// throw loop (e.g. Veldrid "surface lost" thrown every Draw frame while the
// Android Vulkan surface is unavailable during a slow startup) can otherwise
// produce hundreds of full-stack dumps, each one a synchronous file write
// on the throwing thread — which itself stalls the Draw thread and worsens
// the very condition causing the throws.
private const int first_chance_global_cap = 50;

// Per-unique-stack cap. Higher (10) for true fatal kinds caught via
// FirstChanceException-fallback or AppDomain.UnhandledException; lower (3)
// for first-chance noise where seeing the first few occurrences is enough
// to diagnose and the rest are pure log bloat.
private const int per_key_cap_default = 10;
private const int per_key_cap_first_chance = 3;
Comment on lines +40 to +53

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 class-level documentation says managed-exception dumps are written to both internal and external storage "in real time", but the new first-chance path deliberately skips external writes. Update the docs near this new throttling/capping logic to reflect that first-chance exceptions are internal-only and only mirrored to external on next startup.

Copilot uses AI. Check for mistakes.
Comment on lines +46 to +53

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.

Const field naming here (first_chance_global_cap, per_key_cap_*) is inconsistent with the rest of the file (CRASH_LOG_NAME, SENTINEL_NAME) and typical C# conventions. Renaming these constants to match the existing style (e.g., FIRST_CHANCE_GLOBAL_CAP / FirstChanceGlobalCap) would keep the file consistent and improve readability.

Suggested change
private const int first_chance_global_cap = 50;
// Per-unique-stack cap. Higher (10) for true fatal kinds caught via
// FirstChanceException-fallback or AppDomain.UnhandledException; lower (3)
// for first-chance noise where seeing the first few occurrences is enough
// to diagnose and the rest are pure log bloat.
private const int per_key_cap_default = 10;
private const int per_key_cap_first_chance = 3;
private const int FIRST_CHANCE_GLOBAL_CAP = 50;
// Per-unique-stack cap. Higher (10) for true fatal kinds caught via
// FirstChanceException-fallback or AppDomain.UnhandledException; lower (3)
// for first-chance noise where seeing the first few occurrences is enough
// to diagnose and the rest are pure log bloat.
private const int PER_KEY_CAP_DEFAULT = 10;
private const int PER_KEY_CAP_FIRST_CHANCE = 3;

Copilot uses AI. Check for mistakes.

private static string? internalDir;
private static string? externalDir;
private static readonly ConcurrentDictionary<string, int> exceptionCounts = new ConcurrentDictionary<string, int>();
private static int firstChanceWriteCount;
private static string? sentinelPath;
private static string? installedLogPath;
private static bool sentinelWritten;
Expand Down Expand Up @@ -290,8 +306,18 @@ private static void writeManagedException(string source, Exception? ex)
if (ex is EntryPointNotFoundException && ex.Message.Contains("CFStringCreateWithCharacters"))
return;

bool isFirstChance = source.StartsWith("FirstChanceException", StringComparison.Ordinal);

// Global cap on first-chance dumps: a hot-path throw loop on the Draw
// thread can otherwise produce unbounded synchronous file writes, which
// themselves stall the Draw thread and worsen the surface-acquisition
// problem that caused the throws.
if (isFirstChance && Interlocked.Increment(ref firstChanceWriteCount) > first_chance_global_cap)
return;

int perKeyCap = isFirstChance ? per_key_cap_first_chance : per_key_cap_default;
string key = $"{source}_{ex?.GetType().Name}_{ex?.StackTrace?.GetHashCode() ?? 0}";
if (exceptionCounts.AddOrUpdate(key, 1, (_, count) => count + 1) > 10)
if (exceptionCounts.AddOrUpdate(key, 1, (_, count) => count + 1) > perKeyCap)
return;

try
Expand All @@ -306,7 +332,17 @@ private static void writeManagedException(string source, Exception? ex)
"\n" +
(ex?.ToString() ?? "<no exception object>") + "\n" +
"=== END OF MANAGED EXCEPTION ===\n\n";
appendToBoth(block);

// For FirstChanceException we deliberately skip the external/FUSE
// write — those writes are tens of milliseconds each and run on the
// throwing thread (often the Draw thread). MirrorInternalLogToExternal
// copies the internal log to external on the next startup, which is
// sufficient for user-facing diagnostics without risking a Draw-thread
// stall in the live process.
if (isFirstChance)
tryAppend(internalDir, block);
else
appendToBoth(block);
}
catch (Exception e)
{
Expand Down
15 changes: 13 additions & 2 deletions osu.Android/OsuGameActivity.cs
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,19 @@ public OsuGameActivity()

protected override void OnCreate(Bundle? savedInstanceState)
{
// Force orientation immediately to prevent unnecessary surface recreation on startup.
RequestedOrientation = ScreenOrientation.Landscape;
// NOTE: do NOT assign RequestedOrientation here. The `[Activity]` attribute on this
// class already declares `ScreenOrientation = ScreenOrientation.Landscape`, so the
// activity is created in landscape from the very first frame. A runtime re-assignment
// *before* `base.OnCreate` (which is where SDL constructs the SurfaceView) gets
// queued by Android and delivered exactly during initial SurfaceView setup, nudging
// the SurfaceView into a destroy/recreate cycle on some OEMs while the SDL draw
// thread is mid-Vulkan-init. The framework's `VeldridDevice` then either times out
// its 5s `SurfaceHandle` poll (constructor throws, renderer never comes up) or hands
// a stale handle to `vkCreateAndroidSurfaceKHR` (driver SIGSEGV) — either way the
// game never renders a frame and the user is left staring at a black screen while
// the per-frame retry / FirstChanceException pipeline floods the log. The same
// invariant is documented further down in this method (see the "Phones: manifest
// already requests Landscape; do not re-assign at runtime" block).

// Crash diagnostics first. The native handler write target is internal storage
// (FilesDir/native_crash.log); a one-shot mirror copies it to external storage
Expand Down
56 changes: 42 additions & 14 deletions osu.Android/OsuGameAndroid.cs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@
private object? activeMixersList;

private object? nativeBridges;

/// <summary>
/// Last value passed to <see cref="OsuGameActivity.RequestedOrientation"/> by

Check warning on line 93 in osu.Android/OsuGameAndroid.cs

View workflow job for this annotation

GitHub Actions / Build only (Android)

XML comment has cref attribute 'RequestedOrientation' that could not be resolved

Check warning on line 93 in osu.Android/OsuGameAndroid.cs

View workflow job for this annotation

GitHub Actions / Build only (Android)

XML comment has cref attribute 'RequestedOrientation' that could not be resolved

Check warning on line 93 in osu.Android/OsuGameAndroid.cs

View workflow job for this annotation

GitHub Actions / Build only (Android)

XML comment has cref attribute 'RequestedOrientation' that could not be resolved

Check warning on line 93 in osu.Android/OsuGameAndroid.cs

View workflow job for this annotation

GitHub Actions / Build only (Android)

XML comment has cref attribute 'RequestedOrientation' that could not be resolved
/// <see cref="updateOrientation"/>. Cached locally so we can short-circuit
/// redundant updates without round-tripping through the activity getter, which
/// itself performs a binder IPC on modern Android.
/// </summary>
private global::Android.Content.PM.ScreenOrientation? lastRequestedOrientation;
Comment on lines +92 to +98

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.

PR description says the only behavioral change is removing the early RequestedOrientation write in OsuGameActivity.OnCreate, but this PR also adds orientation-update caching here and changes crash-diagnostics throttling. Please align the PR description/scope with the actual changes (or split into separate PRs) so reviewers and release notes don't miss these behavioral changes.

Copilot uses AI. Check for mistakes.
private int currentRefreshRate;

public OsuGameAndroid(OsuGameActivity activity)
Expand Down Expand Up @@ -753,24 +761,44 @@

var orientation = MobileUtils.GetOrientation(this, currentScreen, gameActivity.IsTablet);

global::Android.Content.PM.ScreenOrientation desired;

switch (orientation)
{
case MobileUtils.Orientation.Locked:
desired = global::Android.Content.PM.ScreenOrientation.Locked;
break;

case MobileUtils.Orientation.Portrait:
desired = global::Android.Content.PM.ScreenOrientation.Portrait;
break;

case MobileUtils.Orientation.Default:
desired = gameActivity.DefaultOrientation;
break;

default:
return;
}

// Short-circuit when no change is required. We track the last requested orientation
// locally because Activity.getRequestedOrientation() itself performs a binder IPC
// on modern Android, and the whole point of this guard is to avoid binder traffic.
// ScreenChanged fires on every screen push/pop and the resolved orientation rarely
// differs between adjacent screens, so without this guard we flood the UI looper
// with redundant Activity.setRequestedOrientation transactions, which under
// system_server CPU pressure can wedge input dispatch and trigger an ANR
// ("Input dispatching timed out ... Waited 10000ms for MotionEvent").
if (lastRequestedOrientation == desired)
return;

lastRequestedOrientation = desired;

Comment on lines +792 to +796

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.

lastRequestedOrientation is updated before the UI-thread RequestedOrientation assignment actually succeeds. If setRequestedOrientation throws (you already catch exceptions), this leaves the cache in a state that will permanently short-circuit future retries even though the activity orientation was never updated. Consider only updating lastRequestedOrientation after a successful assignment on the UI thread (or resetting it in the catch path).

Copilot uses AI. Check for mistakes.
gameActivity.RunOnUiThread(() =>
{
try
{
switch (orientation)
{
case MobileUtils.Orientation.Locked:
gameActivity.RequestedOrientation = global::Android.Content.PM.ScreenOrientation.Locked;
break;

case MobileUtils.Orientation.Portrait:
gameActivity.RequestedOrientation = global::Android.Content.PM.ScreenOrientation.Portrait;
break;

case MobileUtils.Orientation.Default:
gameActivity.RequestedOrientation = gameActivity.DefaultOrientation;
break;
}
gameActivity.RequestedOrientation = desired;
}
catch (Exception e)
{
Expand Down
Loading