fix: free a pattern composer's audio in the Android, Flutter and KMP SDKs - #296
Merged
Merged
Conversation
`parsePattern` left `soundPlayer` untouched, so a composer that had been parsed with a sound kept it: the next `play()` took the `player != null` branch and replayed the previous file against the new pattern, skipping the AudioSimulator the sound-free pattern asked for. The player stayed allocated too. iOS already resets its sound state on every parse. Both parse paths now go through `releaseSound()` first, which makes the explicit release in `parsePatternWithSound` redundant.
`PatternComposer_release` called `stop()` and dropped the entry from the registry, so `release()` — the only path that frees the SoundPool or MediaPlayer — became unreachable and every disposed composer leaked one. The iOS handler has always called `dispose()`. The activity-detach hooks cleared the registry the same way.
iOS: `parse()` overwrote the player ids without handing the old players back, leaving two dead players per re-parse in the engine's 20-slot registry — the same leak just fixed in the Swift SDK, and the same release order (players before the audio resource). Android: `AndroidPatternComposerHandle` never overrode `dispose()`, so it inherited the empty default and the native `release()` was unreachable; every composer leaked its SoundPool or MediaPlayer. The handle was also a lazy singleton, so every `getPatternComposer()` shared one native composer and a bundle's presets overwrote each other's parsed state — disposing one would now free another's audio, so the handle is built per call, matching iOS and the native Android SDK.
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.
Follow-up to #294, which fixed the composer leak in React Native and the iOS SDK. Auditing the other SDKs for the same class of bug — a composer's native audio never handed back — turned up four more, in three of them.
Not affected: web (
usePatternComposerkeeps one composer in a ref for the component's lifetime,parseonly mutates it, and there are no native audio handles) and the Dart side of Flutter (PulsarPatternComposercaches_composerIdand passes it back, so both platform handlers reuse one native composer instead of allocating per parse).Android SDK — a re-parsed composer keeps the previous sound
PatternComposer.parsePatternnever touchedsoundPlayer; onlyparsePatternWithSoundreleased it. So on a composer parsed with a sound and then re-parsed without one,play()took theplayer != nullbranch and replayed the previous file against the new pattern, skipping the AudioSimulator the sound-free pattern asked for — and the old player stayed allocated. iOS has always reset its sound state on every parse.Both parse paths now go through
releaseSound()first, which makes the explicit release insideparsePatternWithSoundredundant.Reachable from the native Android API and from Flutter (which reuses one composer across parses). Not from React Native, where every parse gets a fresh composer.
Flutter plugin —
releasedid not release on AndroidPatternComposer_releasecalledstop()and dropped the entry from the registry in one move, sorelease()— the only path that frees theSoundPool/MediaPlayer— became unreachable, and every disposed composer leaked one native audio player. The iOS handler has always calleddispose(). The two activity-detach hooks cleared the registry the same way.Every
HapticLottieController.dispose()andAdaptiveHaptics.dispose()on Android leaked one player.KMP iOS — the same player leak as the Swift SDK
iosMain's composer is an independent Kotlin/Native implementation, and it carries the identical defect #294 fixed in Swift:parse()overwrote the player ids without handing the old players back, leaving two dead players per re-parse in the engine's 20-slot registry, where they can evict a player that is still playing. Same fix, same release order — players before the audio resource.KMP Android —
dispose()was a no-op, over a shared composerAndroidPatternComposerHandlenever overrodedispose(), so it inherited the empty default inPulsarRuntimeand the nativerelease()was unreachable:PatternComposer.dispose(),PresetHandle.dispose()andLoadedBundle.dispose()all did nothing on Android, leaking aSoundPool/MediaPlayereach.The handle was also a
by lazysingleton, so everygetPatternComposer()returned the same native composer. A bundle gives eachPresetHandleits own composer wrapper, so presets overwrote each other's parsed state —A.play(),B.play(),A.play()short-circuits on A's cached parse while the native composer holds B's effect and sound, and A silently plays B. It also made a realdispose()unsafe, since freeing one would free another's audio.The handle is now built per call, matching iOS and the native Android SDK's own
getPatternComposer(), which has always returned a fresh instance.Verification
Android/Pulsar:reparsingWithoutSoundDropsThePreviousSoundfails without the fix, passes with it. Full:Pulsar:testDebugUnitTestgreen.:library:iosSimulatorArm64Testand:library:testAndroidHostTestgreen (withUSE_LOCAL_PULSAR_ANDROID=1— against the published 1.3.0 artifact the KMP and Flutter Android sources do not compile onmaineither, the usual artifact lag).:pulsar_haptics:compileDebugKotlingreen against local sources.soundPlayerbecameinternalwith a private setter so the test can assert it without reflection; it stays invisible outside the module.Deliberately left out
PulsarPatternComposer'sreturn result ?? composerId ?? 0inpulsar_method_channel.dart— on a null native result a first parse yields id0, an id the plugin never allocates. Defensive default, not a leak.pulsar.getPatternComposer()fresh on each button tap and never disposes, so its Stop button builds a third composer and cannot stop what the play button started. A sample-app fix, separate from the SDK.