Repository navigation
fix(player): late midiLoaded registration overflows the stack with the worker player - #2928
Merged
Merged
Conversation
The loadedMidiInfo getter returned itself instead of the backing field, so any late midiLoaded registration recursed until the stack overflowed. Fixes #2862
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
AI-authored disclosure (
alphatab-ai-authored-v1)Portions of this content were authored by an AI agent. The agent has read
AGENTS.md and the human submitter accepts responsibility for
compliance with the rules in that document.
Issues
Fixes #2862
Proposed changes
Registering a
midiLoadedhandler should immediately hand it the current value. With the worker player this instead failed withRangeError: Maximum call stack size exceeded.The value is read through a chain:
AlphaTabApiBase.midiLoaded→AlphaSynthWrapper.loadedMidiInfo→AlphaSynthWebWorkerApi.loadedMidiInfo. The last getter returned itself (this.loadedMidiInfo) instead of the backing field_loadedMidiInfo, so every read recursed. The field is set correctly when the worker reportsalphaSynth.midiLoaded; it was just never read. The getter has been wrong since #2284. The API and wrapper layers are correct, so the fix is only this getter.Registering before the first load seemed to work only because, with
enablePlayer: true(automatic player mode), the worker player is attached once a score exists. Before that, the wrapper has no instance and never reaches the broken getter.The same class is also used as the worker player on C# (
ManagedUiFacade) and Android (AndroidUiFacade). There the recursion is a stack overflow that kills the process, so those platforms get the fix too.Checklist
New test
test/audio/AlphaSynthWebWorkerApi.test.ts: it simulates the worker's MIDI-loaded message, then registersmidiLoadedthroughAlphaSynthWrapper. It checks that the handler is called once with the loaded values. It fails with the reported recursion before the fix. It is marked@target webbecause it passes worker messages as plain objects, which can't be built in the C#/Kotlin tests. C# and Kotlin builds pass with the changed source.AI authorship disclosure
(
alphatab-ai-authored-v1) is present at the top of this body, and I havepersonally reviewed every change and can explain each one
Further details