Fix Android startup crash: Linux libbass.so packaged in place of Android arm64 binary - #224
Conversation
Fixes ppy#37434 --------- Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
) - Supersedes / closes ppy#18129. Reasons I didn't use that PR are hopefully obvious upon comparing diffs but I can elaborate if they are not. - Single metric included for demonstration purposes. - Do not want to talk about further schema design at this time. - Specify `OSU_WEBSOCKET_SERVER=1` envvar to enable. - Can test consumption with [this five minute html job](https://github.com/user-attachments/files/26839923/index.html) (works even as a standalone file opened in browser, no CORS bs!) - There's a lot of inline comments, go read them. There are many WTFs because the .NET frozen websocket API is weird and stanky and reeks of the year 2007. The inline comments attempt to explain.
Due to the push of the relevant screens being delayed it's possible that the room goes away between the scheduling of the push and the actual execution of the push. This maybe closes ppy#37374 but my hopes are not high. Includes some extra cleanups I noticed along the way.
RFC. The reason I'm bringing this up is ppy#37383. In this case, the retrieval of the beatmap failed on a timeout: ``` 2026-04-19 02:43:10 [verbose]: Request to https://osu.ppy.sh/api/v2/beatmaps/?ids[]=5090069 failed with System.Net.WebException: Request to https://osu.ppy.sh/api/v2/beatmaps/?ids[]=5090069 timed out after 10 seconds idle (read 0 bytes, retried 0 times).. 2026-04-19 02:43:10 [verbose]: Failing request osu.Game.Online.API.Requests.GetBeatmapsRequest (System.Net.WebException: Request to https://osu.ppy.sh/api/v2/beatmaps/?ids[]=5090069 timed out after 10 seconds idle (read 0 bytes, retried 0 times).) ``` ``` 2026-04-19 02:43:10 [error]: Failed to load beatmap 5090069 for playlistItem 0. 2026-04-19 02:43:10 [verbose]:⚠️ Failed to load beatmap 5090069 for playlistItem 0. ``` This fails at https://github.com/ppy/osu/blob/0e9664bfdfa69b4b26ff9cf84615c4b83a195a0e/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Card/RankedPlayCard.cs#L167-L171 Just this failing wouldn't cause the game to soft-lock; when I suppress the lookup locally, the game continues as normal. *However*, suppressing the lookup is not the same as the lookup *failing*, because a failed lookup will write a null to the cache, which means that when a beatmap download is initiated later in https://github.com/ppy/osu/blob/1b488949e13568c23faa0d88b18c8036f4a7dbc8/osu.Game/Screens/OnlinePlay/Matchmaking/Match/ScreenMatchmaking.cs#L308-L320 it'll just do nothing and the user will be stuck, even though they could very well attempt to download the beatmap here if the lookup were to succeed on the second go. To a degree changing the whole cache for this could be viewed as the tail wagging the dog, but I think in general caching nulls here seems pretty anti-user. From within the client I would generally not expect very many complete misses when looking up beatmaps. Note that this wouldn't soft-lock the match anymore since ppy/osu-server-spectator#471. It's just another step in making degradation less impactful.
…k around framework breakage (ppy#37453) Similar idea to ppy@622216d (which was included in ppy#37218). Probably closes ppy#37420. I wouldn't be myself if I didn't remark that the WTF/sec on ranked play code continues to be quite high as I fix these issues. Like, look at the old code: why does `Enabled` becoming false stop the preview track, but `CardHovered` becoming false *doesn't*? Even though `shouldBePlaying` was explicitly defined as derived from *both flags*? Maybe me changing this to be actually consistent incurs a behaviour change, but like... I can't tell if it is a bug or not. Not to mention this nugget: https://github.com/ppy/osu/blob/0e9664bfdfa69b4b26ff9cf84615c4b83a195a0e/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.HandCard.cs#L107 https://github.com/ppy/osu/blob/0e9664bfdfa69b4b26ff9cf84615c4b83a195a0e/osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Card/RankedPlayCard.cs#L47-L50 What is this naming even?
…ds` (ppy#37423) Attempt at adressing the points made in ppy#37419 (comment) - `CardContainer` is now being sorted immediately instead of only doing it once per frame. Given that its only ever gonna have a handful of children there wasn't really a need to optimize that that in the first place. - `HandOfCards.Cards` now exposes `cardLookup.Values` as an `IEnumerable` instead of exposing the card container's children directly. - `HandOfCard` now exposes `GetCardsInDisplayOrder` which returns a copy of all cards in display order. Since it's making a copy I made sure this isn't called on any hot code paths. - `HandOfCard.Clear` previously didn't clear the `cardLookup` dictionary. Didn't cause any issues since we're not re-adding cards to the hand anywhere but not good regardless. Switched to looping over all cards and calling `RemoveCard` to make sure changes to the removal logic can't get overlooked there again. --------- Co-authored-by: Bartłomiej Dach <dach.bartlomiej@gmail.com>
… server skeleton Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…eam master Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/1e7cb6f9-16ce-458e-921d-22823a0b1260 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
|
|
There was a problem hiding this comment.
Pull request overview
Fixes an Android launch crash caused by incorrectly packaging desktop (glibc-linked) libbass*.so binaries into the APK, and adds CI verification to prevent regressions. The PR also includes upstream changes around ranked-play/matchmaking robustness and introduces an optional desktop WebSocket server for external integrations.
Changes:
- Restrict Android publish-time
.soclassification/removal to prevent non-Android runtime natives from being packed intolib/<abi>/, and add release workflow verification for glibc symbol leakage. - Harden ranked-play/matchmaking screens against unresolved users and improve hand-of-cards ordering/keyboard navigation; adjust queue screen scheduling to avoid delayed push after leaving a room.
- Add a localhost-only WebSocket server/channel plus desktop wiring (env-var gated) and IPC tests.
Reviewed changes
Copilot reviewed 25 out of 25 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| osu.Android.props | Filters out non-Android runtime .so files from publish set; only marks Android runtime assets as native. |
| .github/workflows/release.yml | Enhances APK verification to detect glibc-linked libbass*.so via GLIBC_ symbol checks. |
| osu.Game/Online/API/Requests/Responses/APIUser.cs | Adds APIUser.UnknownUser(int) helper for consistent fallback users. |
| osu.Game/Screens/Play/Leaderboards/MultiplayerLeaderboardProvider.cs | Uses APIUser.UnknownUser() fallback when lookup misses. |
| osu.Game/Screens/OnlinePlay/Multiplayer/MultiplayerMatchSubScreen.cs | Uses APIUser.UnknownUser() fallback for roll events. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/RankedPlayScreen.cs | Adds safe fallbacks for local/opponent user resolution. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Intro/IntroScreen.cs | Avoids First() exceptions when user lookup fails; provides fallbacks. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/HandOfCards.cs | Refactors card enumeration/sorting and layout invalidation; introduces display-order helper. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Hand/PlayerHandOfCards.cs | Updates keyboard selection to respect display order and ignore modifier combos. |
| osu.Game/Screens/OnlinePlay/Matchmaking/RankedPlay/Card/RankedPlayCard.SongPreview.cs | Refactors song preview container; removes hover/enabled-driven playback wiring (needs attention). |
| osu.Game/Screens/OnlinePlay/Matchmaking/Queue/ScreenQueue.cs | Fire-and-forget matchmaking lobby calls; cancels delayed screen push when state changes. |
| osu.Game/Screens/OnlinePlay/Matchmaking/Queue/QueueController.cs | Removes outdated remarks. |
| osu.Game/Database/BeatmapLookupCache.cs | Disables caching of null lookups. |
| osu.Game/IPC/WebSocketServer.cs | Adds simple localhost-only WebSocket server built on HttpListener. |
| osu.Game/IPC/WebSocketChannel.cs | Adds WebSocket read/write loop with message size enforcement. |
| osu.Desktop/Program.cs | Adds env-var flag to enable WebSocket server. |
| osu.Desktop/OsuGameDesktop.cs | Wires optional OsuWebSocketProvider into desktop game. |
| osu.Desktop/IPC/OsuWebSocketProvider.cs | Broadcasts hitcount updates over WebSocket. |
| osu.Desktop/IPC/Messages/OsuWebSocketMessage.cs | Base message type with type discriminator. |
| osu.Desktop/IPC/Messages/HitCountMessage.cs | Message payload for incremental hit count updates. |
| osu.Game.Tests/Visual/RankedPlay/TestSceneRankedPlayScreen.cs | Adjusts setup and adds unresolved-user coverage. |
| osu.Game.Tests/Visual/RankedPlay/TestScenePlayerCardHand.cs | Updates tests to use display-order list accessor. |
| osu.Game.Tests/Visual/Matchmaking/TestSceneMatchmakingQueueScreen.cs | Adds regression test for delayed room screen push cancellation. |
| osu.Game.Tests/IPC/WebSocketTest.cs | Adds integration-style tests for WebSocket server/channel. |
| osu.Game.Tests/IPC/WebSocketClient.cs | Test client helper for WebSocket IPC tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -126,9 +97,6 @@ public void LoadPreview(APIBeatmap beatmap) | |||
| { | |||
| TrackRunning = { BindTarget = trackRunning } | |||
| }); | |||
|
|
|||
| if (shouldBePlaying) | |||
| startPreviewIfAvailable(); | |||
| }); | |||
| } | |||
There was a problem hiding this comment.
LoadPreview() no longer starts playback when Enabled/CardHovered are true, and the removed BindValueChanged handlers mean CardHovered/Enabled no longer control playback at all. Since RankedPlayCard.PlayAudioPreview still drives songPreviewContainer.CardHovered, this effectively disables (or leaves stale) song previews. Please reintroduce start/stop logic (binding to Enabled and CardHovered), or alternatively remove these bindables and make the caller explicitly control PreviewTrack.Start()/Stop().
| if (currentBufferPosition >= max_message_size) | ||
| { | ||
| await webSocket.CloseOutputAsync(WebSocketCloseStatus.MessageTooBig, $@"Exceeded maximum message size of {max_message_size} bytes.", token).ConfigureAwait(false); | ||
| ClosedPrematurely?.Invoke(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
The max message size check rejects messages of exactly max_message_size bytes (currentBufferPosition >= max_message_size). The docstring says messages must not exceed the limit, so a message with length exactly 4096 bytes should be accepted (and the buffer is sized for it). Adjust the condition to only fail when the message would exceed the limit (taking EndOfMessage into account).
| try | ||
| { | ||
| message = Encoding.UTF8.GetString(receiveBuffer, 0, currentBufferPosition); | ||
| } | ||
| catch (ArgumentException) | ||
| { | ||
| await webSocket.CloseOutputAsync(WebSocketCloseStatus.InvalidPayloadData, @"UTF-8 encoded strings expected.", token).ConfigureAwait(false); | ||
| ClosedPrematurely?.Invoke(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Encoding.UTF8.GetString() will not throw for invalid UTF-8 sequences (it uses replacement fallback by default), so the catch (ArgumentException) path won't detect malformed payloads as intended. If you want to enforce UTF-8 validity and close with InvalidPayloadData, decode using a UTF8Encoding(..., throwOnInvalidBytes: true) instance and catch DecoderFallbackException (or similar).
| public void Dispose() | ||
| { | ||
| if (isDisposed) | ||
| return; | ||
|
|
||
| isDisposed = true; | ||
|
|
||
| // no clue why this isn't accessible without casting. | ||
| // sidebar: `Stop()` unregisters addresses on Windows, but `Abort()` doesn't! | ||
| // this `Dispose()` implementation calls the former. | ||
| (listener as IDisposable)?.Dispose(); | ||
|
|
||
| foreach (var channel in channels.Values) | ||
| channel.Dispose(); | ||
|
|
||
| runningTokenSource.Dispose(); | ||
| contextResetEvent.Dispose(); | ||
| } |
There was a problem hiding this comment.
Dispose() disposes runningTokenSource/contextResetEvent and the HttpListener without first ensuring the request loop has exited. If Dispose() is called without a prior StopAsync(), handleRequests() can still be running and may access runningTokenSource.Token.WaitHandle / contextResetEvent, leading to ObjectDisposedException or other undefined behaviour. Consider making Dispose() reliably stop the server (cancel runningTokenSource, stop/close the listener, and wait for handleRequestTask), or at least call into StopAsync() with a bounded timeout before disposing resources.
| if (server?.IsRunning == true) | ||
| { | ||
| var cts = new CancellationTokenSource(); | ||
| cts.CancelAfter(TimeSpan.FromSeconds(10)); | ||
| server.StopAsync(cts.Token).WaitSafely(); | ||
| server = null; | ||
| } |
There was a problem hiding this comment.
Dispose() creates a CancellationTokenSource but never disposes it, and also never calls Dispose() on the WebSocketServer after stopping it. This can leak unmanaged handles (listener, wait handles) across screen transitions/shutdown. Use a using/try-finally for the CTS and ensure the server is disposed after stopping (even if stop fails/times out).
| if (server?.IsRunning == true) | |
| { | |
| var cts = new CancellationTokenSource(); | |
| cts.CancelAfter(TimeSpan.FromSeconds(10)); | |
| server.StopAsync(cts.Token).WaitSafely(); | |
| server = null; | |
| } | |
| var server = this.server; | |
| this.server = null; | |
| if (server == null) | |
| return; | |
| try | |
| { | |
| if (server.IsRunning) | |
| { | |
| using var cts = new CancellationTokenSource(); | |
| cts.CancelAfter(TimeSpan.FromSeconds(10)); | |
| server.StopAsync(cts.Token).WaitSafely(); | |
| } | |
| } | |
| finally | |
| { | |
| server.Dispose(); | |
| } |
| const int port = 54321; | ||
|
|
||
| var server = new WebSocketServer(port); | ||
| var client = new WebSocketClient(port); | ||
|
|
There was a problem hiding this comment.
All tests in this fixture hardcode the same port (54321). This can make the suite flaky if the port is already in use on the runner, or if multiple test processes/assemblies execute concurrently. Consider selecting a free ephemeral port at runtime (e.g., bind a TcpListener on port 0 to reserve a port, read it, then start the server) and passing that into both server/client.
… + README note Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/46b5e755-967f-4136-b92d-96697f1ecd3b Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/46b5e755-967f-4136-b92d-96697f1ecd3b Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…dary/Dispose Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/a7e9baba-d7f9-4d66-bac6-a6540c230360 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
…ider reference Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/1fd19d7e-9231-44c6-af3c-c266fbf170f3 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
|
Your attempts at cosplaying software development are causing continuous annoyance and disruption to our processes. Your robot army's "PRs" keep getting linked to ours, your attempts at tagging releases have required us to yank sentry keys, and from a cursory look through your "development history" you're still having trouble getting anything to run at all which is not surprising in the slightest. For the interest of our sanity and presumably your wallet, I implore you to stop and delete this repository. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
…ebSocketServer.syncRoot Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/c8ad2ddc-3e69-424b-aa69-46b457a9df9b Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Crash root cause
System.DllNotFoundException: bassatAudioManagerstartup (v143 / 2026.421.143).The APK shipped
lib/arm64-v8a/libbass.soat 261160 bytes — the Linux arm64 binary fromppy.osu.Framework.NativeLibs(runtimes/linux-arm64/native/libbass.so), not the Android arm64 binary (322992 bytes) fromppy.osu.Framework.Android's AAR. Same wrong-binary substitution forlibbass_fx.so(85536 vs 104344) andlibbassmix.so(53360 vs 59072). The Linux ELF referencesGLIBC_*versioned symbols; bionic can't resolve them.FixRuntimePackAssetTypesinosu.Android.propsindiscriminately marked every.soinRuntimePackAsset/ResolvedFileToPublishasAssetType=native, so the .NET Android SDK packedruntimes/linux-arm64/native/libbass.sointolib/arm64-v8a/, racing with — and replacing — the AAR copy. Other natives were spared because their Linux variants carry.so.58suffixes and didn't collide by name. The release-workflow verify step matched on filename only, so the swap slipped past CI.Checklist
Packaging fix (the actual launch crash)
osu.Android.props—FixRuntimePackAssetTypesonly marks Android-RID.sofiles as native; explicitly removes desktop/iOS runtime.sofiles (runtimes/{linux,osx,ios,maccatalyst,win,browser,freebsd}-*/) fromResolvedFileToPublish. Filter is name-agnostic.release.yml— Verify step extracts each bass.soand rejects the build if it containsGLIBC_*versioned symbols.Upstream merge
ppy/osumaster commits (Implement WebSocket server skeleton for external integrations ppy/osu#37335 WebSocket server, Code quality improvements for child/draw order handling inHandOfCardsppy/osu#37423 HandOfCards code-quality, Fix ranked play chat deselecting when typing shift-numbers ppy/osu#37450 chat shift-numbers, Attempt to improve safety of pushing matchmaking screens ppy/osu#37452 / Use placeholder user models in ranked play if online lookups fail ppy/osu#37455 / Do not cache null values in beatmap lookup cache ppy/osu#37456 matchmaking safety, Rewrite ranked play card song preview playback logic to hopefully work around framework breakage ppy/osu#37453 song-preview rewrite).PlayerHandOfCards.OnKeyDown: wrapped theSpacecase body in its own{ … }.Copilot AI review feedback
RankedPlayCard.SongPreviewContainer— re-wiredEnabled/CardHovered→Start()/Stop()lost in upstream's Rewrite ranked play card song preview playback logic to hopefully work around framework breakage ppy/osu#37453 rewrite. Bind happens inside theLoadComponentAsynccontinuation so previews never race the track's async load.WebSocketChannel.max_message_sizeboundary —>=→>, so a payload of exactly 4096 bytes is accepted.WebSocketChannelUTF-8 validation — switched to a staticUTF8Encoding(throwOnInvalidBytes: true)so malformed payloads actually trigger theInvalidPayloadDataclose path; catchDecoderFallbackException.WebSocketServer.Dispose— cancelsrunningTokenSource, disposes the listener, then waits up to 2 s forhandleRequestTaskto exit before disposing the cancellation/reset-event handles.OsuWebSocketProvider.Dispose— local-swapserver, dispose theCancellationTokenSourceviausing, always dispose the server infinally.WebSocketTest.cshardcoded port 54321 — tests-only, low value, would diverge from upstream.CI sanity check + CodeQL / GitHub Advanced Security
bc9b8aea:CS0246: 'OsuWebSocketProvider' could not be foundinosu.Desktop/OsuGameDesktop.cs(155,25)— fixed by addingusing osu.Desktop.IPC;(alphabetically ordered amongst theosu.Desktop.*usings to satisfy InspectCode).WebSocketServer.syncRoottyped asobject— changed toSystem.Threading.Lock(= new Lock()). Repo-wide convention verified across 9 sites (OnlineLookupCache,ScreenshotManager,APIRequest,OAuth,TaskChain,BeatmapDifficultyCache,WorkingBeatmap,SkinProvidingContainer,SubmittingPlayer).System.Threadingis already imported.timeout-minutes: 120accommodates).dotnet buildnot possible in this sandbox (GitHub Packages auth is sandboxed away) — relying on CI for full validation.Full-code sanity pass over PR diff
PlayerHandOfCards.cs— brace fix properly scopes thecardlocal to theSpacecase; no duplicate decls elsewhere.WebSocketChannel.cs—strict_utf8followsprivate static readonly <Type> <snake_case>convention (verified across ~50 sites).WebSocketServer.Dispose()— touches only fields always initialized (runningTokenSource,handleRequestTask,contextResetEvent,listener); 2 s bounded wait prevents shutdown hang.OsuWebSocketProvider.Dispose()— null-swap +using var cts+finally server.Dispose(); idempotent ifDispose()is called twice.OsuGameDesktop—using osu.Desktop.IPC;in place;EnableWebSocketServergate unchanged.HandOfCards, localhost-only IPC,StopAsync). Real wins (HitObject.DefaultsAppliedleak,MemoryCachingComponentbounding) are too invasive for this PR — deferred.README
Out of scope / follow-ups
HitObject.DefaultsAppliedevent leak (HitObjectLifetimeEntry) — real, but a proper fix needs anIDisposableflow on the entry plus rewiring every entry-owning Playfield/HitObjectContainer; queued for a separate PR.MemoryCachingComponentunbounded cache → LRU. Lives inosu-frameworksubmodule, not in this PR's scope.