Skip to content

Commit b39b878

Browse files
authored
Merge ppy/master: catch up ranked play audio preview workaround (ppy#37477)
- Merge upstream ppy/osu master through commit a4f79f7 (PR ppy#37477: ranked play single-thread audio fix, supersedes/reverts ppy#37463). - Resolve conflict in RankedPlayCard.SongPreview.cs in favour of our fork's existing late-bind approach (Enabled/CardHovered subscribed inside LoadComponentAsync callback so they never fire pre-load). - Pick up the new TestPreviewStopsOnEnteringGameplay regression test. Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
2 parents b519f0c + a4f79f7 commit b39b878

5 files changed

Lines changed: 102 additions & 15 deletions

File tree

osu.Android.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
</PropertyGroup>
5252

5353
<ItemGroup>
54-
<PackageReference Include="ppy.osu.Framework.Android" Version="2026.420.2" />
54+
<PackageReference Include="ppy.osu.Framework.Android" Version="2026.422.1" />
5555
<!-- `ppy.osu.Framework.NativeLibs` is a transitive dependency of `ppy.osu.Framework`
5656
that ships desktop-only natives (Linux/macOS/Windows) under `runtimes/<rid>/native/`
5757
— including a bare Linux `libbass.so`/`libbass_fx.so`/`libbassmix.so` for linux-arm64.

osu.Android/OsuGameActivity.cs

Lines changed: 60 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -359,13 +359,32 @@ await Task.WhenAll(uris.Select(async uri =>
359359
});
360360

361361
private readonly System.Threading.ManualResetEventSlim surfaceEvent = new System.Threading.ManualResetEventSlim(false);
362+
363+
// Hold both the JNI global ref AND the managed Surface peer alive against the
364+
// SurfaceView lifecycle. The global ref alone is NOT enough — .NET-for-Android
365+
// tracks managed peers separately, and once the local `Surface` returned by
366+
// `holder.Surface` becomes GC-eligible (i.e. once SurfaceCreated returns), the
367+
// peer's finaliser will release the underlying Java Surface even though we still
368+
// hold a global ref to its handle. The next time the SDL thread tries to use that
369+
// handle through JNI we crash with SIGSEGV inside libart.so on the SDLActivity
370+
// thread (see native_crash.log). Storing the wrapper in a managed field roots the
371+
// peer for the SurfaceView's entire lifetime.
372+
//
373+
// SurfaceCreated and SurfaceDestroyed are serialised against each other via
374+
// `surfaceLock` so the SDL/Veldrid backend can never observe a half-torn-down
375+
// state (e.g. global ref present but managed peer already released, or vice
376+
// versa). The handle reader uses Volatile.Read for an unlocked fast path on hot
377+
// call sites and a locked slow path is unnecessary because all writes happen
378+
// under the lock and Interlocked.Exchange / Volatile.Write are release barriers.
379+
private readonly object surfaceLock = new object();
380+
private global::Android.Views.Surface? heldSurface;
362381
private IntPtr surfaceGlobalRef;
363382

364383
public IntPtr GetSurfaceGlobalRef()
365384
{
366385
if (!surfaceEvent.Wait(5000))
367386
Debug.WriteLine("[osu!] Warning: Wait for surface timed out");
368-
return surfaceGlobalRef;
387+
return System.Threading.Volatile.Read(ref surfaceGlobalRef);
369388
}
370389

371390
public SurfaceView? GetSurface() => findSurfaceView(Window?.DecorView);
@@ -387,19 +406,33 @@ public IntPtr GetSurfaceGlobalRef()
387406
public void SurfaceCreated(ISurfaceHolder holder)
388407
{
389408
var surface = holder.Surface;
390-
if (surface != null && surface.IsValid)
391-
{
392-
IntPtr handle = surface.Handle;
393-
if (handle == IntPtr.Zero) return;
409+
if (surface == null || !surface.IsValid)
410+
return;
411+
412+
IntPtr handle = surface.Handle;
413+
if (handle == IntPtr.Zero)
414+
return;
394415

395-
IntPtr newRef = global::Android.Runtime.JNIEnv.NewGlobalRef(handle);
416+
IntPtr newRef = global::Android.Runtime.JNIEnv.NewGlobalRef(handle);
417+
418+
lock (surfaceLock)
419+
{
420+
// Establish the new managed root BEFORE publishing the new global ref so
421+
// that any reader that observes the new ref already has its managed peer
422+
// pinned. Then atomically swap in the new ref and release the previous one.
423+
var oldHeld = heldSurface;
424+
heldSurface = surface;
396425

397-
// Atomically swap the old reference to prevent race with SurfaceDestroyed.
398426
IntPtr oldRef = System.Threading.Interlocked.Exchange(ref surfaceGlobalRef, newRef);
399427

400428
if (oldRef != IntPtr.Zero)
401429
global::Android.Runtime.JNIEnv.DeleteGlobalRef(oldRef);
402430

431+
// Drop the previous managed root only AFTER its global ref is gone, so
432+
// there is no window where consumers can hold a stale global ref pointing
433+
// into a Java peer whose .NET wrapper has been disposed.
434+
oldHeld?.Dispose();
435+
403436
Debug.WriteLine("[osu!] Native surface JNI global reference created (waiting for SurfaceChanged for signal)");
404437
}
405438
}
@@ -420,12 +453,28 @@ public void SurfaceChanged(ISurfaceHolder holder, global::Android.Graphics.Forma
420453

421454
public void SurfaceDestroyed(ISurfaceHolder holder)
422455
{
423-
IntPtr oldRef = System.Threading.Interlocked.Exchange(ref surfaceGlobalRef, IntPtr.Zero);
456+
// Block any concurrent SurfaceCreated so the SDL/Veldrid thread can never
457+
// observe a partial state where the global ref has been freed but the
458+
// managed peer is still alive (or the inverse).
459+
lock (surfaceLock)
460+
{
461+
// Reset the readiness signal first so any waiter blocks until a new
462+
// surface is published, rather than racing with the teardown below.
463+
surfaceEvent.Reset();
464+
465+
// Release the global ref BEFORE dropping the managed root, never the
466+
// other way around: once the .NET wrapper is disposed the underlying
467+
// Java Surface may be released, and any subsequent JNI use of an
468+
// outstanding global ref to that handle would segfault. Order here
469+
// mirrors the inverse of SurfaceCreated.
470+
IntPtr oldRef = System.Threading.Interlocked.Exchange(ref surfaceGlobalRef, IntPtr.Zero);
424471

425-
if (oldRef != IntPtr.Zero)
426-
global::Android.Runtime.JNIEnv.DeleteGlobalRef(oldRef);
472+
if (oldRef != IntPtr.Zero)
473+
global::Android.Runtime.JNIEnv.DeleteGlobalRef(oldRef);
427474

428-
surfaceEvent.Reset();
475+
heldSurface?.Dispose();
476+
heldSurface = null;
477+
}
429478
}
430479

431480
public override void OnConfigurationChanged(Configuration newConfig)

osu.Game.Tests/Visual/RankedPlay/TestSceneRankedPlayScreen.cs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,11 @@
88
using osu.Game.Graphics.UserInterface;
99
using osu.Game.Online.API;
1010
using osu.Game.Online.API.Requests.Responses;
11+
using osu.Game.Online.Multiplayer;
1112
using osu.Game.Online.Multiplayer.MatchTypes.RankedPlay;
1213
using osu.Game.Online.Rooms;
1314
using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay;
15+
using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Card;
1416
using osu.Game.Screens.OnlinePlay.Matchmaking.RankedPlay.Hand;
1517
using osuTK.Input;
1618

@@ -214,5 +216,41 @@ public void TestHealthChange()
214216
AddWaitStep("wait", 5);
215217
AddStep("change player 2 health", () => MultiplayerClient.RankedPlayChangeUserState(2, state => state.Life = 250_000).WaitSafely());
216218
}
219+
220+
[Test]
221+
public void TestPreviewStopsOnEnteringGameplay()
222+
{
223+
AddStep("join other user", () => MultiplayerClient.AddUser(new APIUser { Id = 2 }));
224+
225+
AddStep("load screen", () => LoadScreen(screen = new RankedPlayScreen(MultiplayerClient.ClientRoom!)));
226+
227+
var requestHandler = new BeatmapRequestHandler();
228+
229+
AddStep("setup request handler", () => ((DummyAPIAccess)API).HandleRequest = requestHandler.HandleRequest);
230+
231+
AddStep("set play phase", () => MultiplayerClient.RankedPlayChangeStage(RankedPlayStage.CardPlay, state => state.ActiveUserId = 1001).WaitSafely());
232+
233+
for (int i = 0; i < 3; i++)
234+
{
235+
int i2 = i;
236+
AddStep("reveal card", () => MultiplayerClient.RankedPlayRevealCard(hand => hand[i2], new MultiplayerPlaylistItem
237+
{
238+
ID = i2,
239+
BeatmapID = requestHandler.Beatmaps[i2].OnlineID
240+
}).WaitSafely());
241+
}
242+
243+
AddStep("hover first card", () => InputManager.MoveMouseTo(this.ChildrenOfType<PlayerHandOfCards>().Single().Cards.First()));
244+
AddUntilStep("preview playing", () => this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().Any(p => p.IsRunning), () => Is.True);
245+
246+
AddWaitStep("wait", 1);
247+
AddStep("play beatmap", () => MultiplayerClient.PlayUserCard(1001, hand => hand[0]).WaitSafely());
248+
249+
AddStep("set warmup", () => MultiplayerClient.RankedPlayChangeStage(RankedPlayStage.GameplayWarmup).WaitSafely());
250+
AddUntilStep("preview running", () => this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().Any(p => p.IsRunning), () => Is.True);
251+
252+
AddStep("load requested", () => ((IMultiplayerClient)MultiplayerClient).LoadRequested());
253+
AddUntilStep("preview stopped", () => this.ChildrenOfType<RankedPlayCard.SongPreviewContainer>().Any(p => p.IsRunning), () => Is.False);
254+
}
217255
}
218256
}

osu.Game/osu.Game.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,10 +38,10 @@
3838
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
3939
</PackageReference>
4040
<PackageReference Include="Realm" Version="20.1.0" />
41-
<PackageReference Include="ppy.osu.Framework" Version="2026.420.2" />
41+
<PackageReference Include="ppy.osu.Framework" Version="2026.422.1" />
4242
<!--
4343
Explicitly pin `ppy.Veldrid.SPIRV` to the winnerspiros fork build that
44-
`ppy.osu.Framework 2026.420.2` was compiled against. This version is the only
44+
`ppy.osu.Framework 2026.422.1` was compiled against.This version is the only
4545
one whose `runtimes/android-arm64/native/libveldrid-spirv.so` is aligned to 16 KB
4646
pages (required by Android 16+). It lives only as a release asset on
4747
<https://github.com/winnerspiros/veldrid-spirv/releases/tag/1.0> and is vendored

osu.iOS.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,6 @@
3333
<SuppressTrimAnalysisWarnings>true</SuppressTrimAnalysisWarnings>
3434
</PropertyGroup>
3535
<ItemGroup>
36-
<PackageReference Include="ppy.osu.Framework.iOS" Version="2026.420.2" />
36+
<PackageReference Include="ppy.osu.Framework.iOS" Version="2026.422.1" />
3737
</ItemGroup>
3838
</Project>

0 commit comments

Comments
 (0)