rock-n-code/playdate-kit
1
0
Fork 0
Code Issues Pull Requests Actions Releases 2 Activity

Fixed the double-free in the "deinit" method of the Sound Effect type in the Playdate bindings target and cover ownership.

Browse Source
This commit is contained in:
javier
2026-07-26 00:45:45 +02:00
parent d233cbaddb
commit d84113f7a8
3 changed files with 251 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Sources/PlaydateKit/Sound/Effect/Classes/Effect.swift
+7 -1
View File
@@ -38,10 +38,16 @@ extension Sound {
}
deinit {
// Subclasses free the C object in their own deinit with the
// subsystem's type-specific free (freeDelayLine, freeOverdrive,
// ...); freeing here as well would double-free. The base class
// owns only the custom-processor effects it creates itself.
if let processorBox {
if isOwned {
effectAPI.pointee.freeEffect.unsafelyUnwrapped(pointer)
}
processorBox?.release()
processorBox.release()
}
}
/// The wet/dry mix: 1 is fully processed, 0 fully dry.
Tests/PlaydateKit/MockPlaydate.swift
+100
View File
@@ -20,6 +20,10 @@ enum Mock {
nonisolated(unsafe) static let spriteAPI = UnsafeMutablePointer<playdate_sprite>.allocate(capacity: 1)
nonisolated(unsafe) static let soundAPI = UnsafeMutablePointer<playdate_sound>.allocate(capacity: 1)
nonisolated(unsafe) static let channelAPI = UnsafeMutablePointer<playdate_sound_channel>.allocate(capacity: 1)
nonisolated(unsafe) static let synthAPI = UnsafeMutablePointer<playdate_sound_synth>.allocate(capacity: 1)
nonisolated(unsafe) static let soundEffectAPI = UnsafeMutablePointer<playdate_sound_effect>.allocate(capacity: 1)
nonisolated(unsafe) static let lfoAPI = UnsafeMutablePointer<playdate_sound_lfo>.allocate(capacity: 1)
nonisolated(unsafe) static let delayLineAPI = UnsafeMutablePointer<playdate_sound_effect_delayline>.allocate(capacity: 1)
nonisolated(unsafe) static let fileAPI = UnsafeMutablePointer<playdate_file>.allocate(capacity: 1)
nonisolated(unsafe) static let jsonAPI = UnsafeMutablePointer<playdate_json>.allocate(capacity: 1)
nonisolated(unsafe) static let apiStruct = UnsafeMutablePointer<PlaydateAPI>.allocate(capacity: 1)
@@ -39,6 +43,19 @@ enum Mock {
nonisolated(unsafe) static var spriteUpdateCallback: (@convention(c) (OpaquePointer?) -> Void)?
nonisolated(unsafe) static var audioCallback: (@convention(c) (UnsafeMutableRawPointer?, UnsafeMutablePointer<Int16>?, UnsafeMutablePointer<Int16>?, Int32) -> Int32)?
nonisolated(unsafe) static var audioContext: UnsafeMutableRawPointer?
/// Generator registrations per synth. The OS deallocs a generator's
/// userdata when the generator is replaced or its synth is freed; the
/// mock mirrors that contract.
struct SynthGenerator {
var render: (@convention(c) (UnsafeMutableRawPointer?, UnsafeMutablePointer<Int32>?, UnsafeMutablePointer<Int32>?, Int32, UInt32, Int32) -> Int32)?
var dealloc: (@convention(c) (UnsafeMutableRawPointer?) -> Void)?
var userdata: UnsafeMutableRawPointer?
}
nonisolated(unsafe) static var synthGenerators: [OpaquePointer: SynthGenerator] = [:]
/// The last custom-effect proc handed to `newEffect`, plus each
/// effect's userdata, for re-invocation by tests.
nonisolated(unsafe) static var effectProc: (@convention(c) (OpaquePointer?, UnsafeMutablePointer<Int32>?, UnsafeMutablePointer<Int32>?, Int32, Int32) -> Int32)?
nonisolated(unsafe) static var effectUserdata: [OpaquePointer: UnsafeMutableRawPointer] = [:]
static func record(_ event: String) {
events.append(event)
@@ -72,6 +89,9 @@ enum Mock {
spriteUpdateCallback = nil
audioCallback = nil
audioContext = nil
synthGenerators = [:]
effectProc = nil
effectUserdata = [:]
}
private static func install() {
@@ -282,6 +302,86 @@ enum Mock {
Mock.record("channelRemoveSource")
return 1
}
channelAPI.pointee.addSource = { _, _ in
Mock.record("channelAddSource")
return 1
}
soundAPI.pointee.synth = UnsafePointer(synthAPI)
synthAPI.initialize(to: playdate_sound_synth())
synthAPI.pointee.newSynth = {
Mock.record("newSynth")
return Mock.fakePointer()
}
synthAPI.pointee.freeSynth = { synth in
Mock.record("freeSynth")
// Freeing a synth deallocs its generator's userdata.
if let synth, let generator = Mock.synthGenerators.removeValue(forKey: synth) {
generator.dealloc?(generator.userdata)
}
}
synthAPI.pointee.setGenerator = { synth, _, render, _, _, _, dealloc, _, userdata in
Mock.record("setGenerator")
guard let synth else { return }
// Replacing a generator deallocs the previous one first.
if let previous = Mock.synthGenerators[synth] {
previous.dealloc?(previous.userdata)
}
Mock.synthGenerators[synth] = SynthGenerator(render: render, dealloc: dealloc,
userdata: userdata)
}
synthAPI.pointee.setFrequencyModulator = { _, _ in
Mock.record("setFrequencyModulator")
}
soundAPI.pointee.lfo = UnsafePointer(lfoAPI)
lfoAPI.initialize(to: playdate_sound_lfo())
lfoAPI.pointee.newLFO = { _ in
Mock.record("newLFO")
return Mock.fakePointer()
}
lfoAPI.pointee.freeLFO = { _ in
Mock.record("freeLFO")
}
soundAPI.pointee.effect = UnsafePointer(soundEffectAPI)
soundEffectAPI.initialize(to: playdate_sound_effect())
soundEffectAPI.pointee.newEffect = { proc, userdata in
Mock.record("newEffect")
let pointer = Mock.fakePointer()
Mock.effectProc = proc
if let userdata {
Mock.effectUserdata[pointer] = userdata
}
return pointer
}
soundEffectAPI.pointee.freeEffect = { effect in
Mock.record("freeEffect")
if let effect {
Mock.effectUserdata[effect] = nil
}
}
soundEffectAPI.pointee.getUserdata = { effect in
guard let effect else { return nil }
return Mock.effectUserdata[effect]
}
soundEffectAPI.pointee.delayline = UnsafePointer(delayLineAPI)
delayLineAPI.initialize(to: playdate_sound_effect_delayline())
delayLineAPI.pointee.newDelayLine = { _, _ in
Mock.record("newDelayLine")
return Mock.fakePointer()
}
delayLineAPI.pointee.freeDelayLine = { _ in
Mock.record("freeDelayLine")
}
delayLineAPI.pointee.addTap = { _, _ in
Mock.record("addTap")
return Mock.fakePointer()
}
delayLineAPI.pointee.freeTap = { _ in
Mock.record("freeTap")
}
}
// MARK: - File
Tests/PlaydateKit/WrapperTests.swift
+142
View File
@@ -182,6 +182,148 @@ struct WrapperTests {
#expect(Sound.CallbackSource.live.count == baseline)
}
// MARK: Sound ownership graph
@Test func channelRetainsAddedSourceUntilRemoved() {
let channel = Sound.Channel()
weak var weakSynth: Sound.Synth?
do {
let synth = Sound.Synth()
weakSynth = synth
channel.addSource(synth)
}
#expect(weakSynth != nil) // the channel keeps the source alive
#expect(Mock.eventCount("freeSynth") == 0)
if let synth = weakSynth {
channel.removeSource(synth)
}
#expect(weakSynth == nil)
#expect(Mock.eventCount("freeSynth") == 1)
}
@Test func synthRetainsItsModulatorWhileAlive() {
weak var weakLFO: Sound.LFO?
do {
let synth = Sound.Synth()
do {
let lfo = Sound.LFO()
weakLFO = lfo
synth.frequencyModulator = lfo
}
#expect(weakLFO != nil) // the synth retains the modulator
#expect(Mock.eventCount("freeLFO") == 0)
}
#expect(weakLFO == nil)
#expect(Mock.eventCount("freeLFO") == 1)
}
@Test func synthGeneratorDispatchesAndIsReleasedWithTheSynth() {
final class Token {}
weak var weakToken: Token?
var rendered = 0
do {
let synth = Sound.Synth()
let token = Token()
weakToken = token
synth.setGenerator(stereo: false, .init(render: { left, _, _, _ in
_ = token
rendered += 1
return left.count
}))
// Simulate the audio engine rendering through the trampoline.
let generator = try! #require(Mock.synthGenerators[synth.pointer])
var samples = [Int32](repeating: 0, count: 8)
let frames = samples.withUnsafeMutableBufferPointer { buffer in
generator.render?(generator.userdata, buffer.baseAddress, nil,
Int32(buffer.count), 0, 0)
}
#expect(frames == 8)
#expect(rendered == 1)
#expect(weakToken != nil)
}
// freeSynth deallocs the generator userdata, releasing the box and
// the closure's captures with it.
#expect(Mock.eventCount("freeSynth") == 1)
#expect(weakToken == nil)
}
@Test func replacingASynthGeneratorReleasesThePreviousOne() {
final class Token {}
weak var firstToken: Token?
let synth = Sound.Synth()
do {
let token = Token()
firstToken = token
synth.setGenerator(stereo: false, .init(render: { left, _, _, _ in
_ = token
return left.count
}))
}
#expect(firstToken != nil)
synth.setGenerator(stereo: false, .init(render: { left, _, _, _ in left.count }))
#expect(firstToken == nil) // the OS deallocs the replaced generator
}
@Test func effectProcessorDispatchesAndIsReleasedOnDeinit() {
final class Token {}
weak var weakToken: Token?
var processed = 0
do {
let token = Token()
weakToken = token
let effect = Sound.Effect(processor: { left, right, _ in
_ = token
processed += left.count
#expect(right == nil)
return true
})
// Drive the effect the way the OS would: through the registered
// proc, which recovers the box from the effect's userdata.
var samples = [Int32](repeating: 0, count: 4)
let active = samples.withUnsafeMutableBufferPointer { buffer in
Mock.effectProc?(effect.pointer, buffer.baseAddress, nil,
Int32(buffer.count), 1)
}
#expect(active == 1)
#expect(processed == 4)
}
#expect(Mock.eventCount("freeEffect") == 1)
#expect(weakToken == nil)
}
@Test func effectSubclassIsFreedExactlyOnceWithItsOwnFree() {
do {
let line = Sound.DelayLine(length: 256)
_ = line
}
#expect(Mock.eventCount("freeDelayLine") == 1)
// The base Effect deinit must not also free the subclass's object.
#expect(Mock.eventCount("freeEffect") == 0)
}
@Test func delayLineTapKeepsItsDelayLineAlive() {
weak var weakLine: Sound.DelayLine?
var tap: Sound.DelayLineTap?
do {
let line = Sound.DelayLine(length: 256)
weakLine = line
tap = line.addTap(delay: 128)
}
#expect(weakLine != nil) // the tap retains its delay line
#expect(Mock.eventCount("freeDelayLine") == 0)
tap = nil
#expect(Mock.eventCount("freeTap") == 1)
#expect(Mock.eventCount("freeDelayLine") == 1)
}
// MARK: File
@Test func fileHandleReadsWritesAndClosesExactlyOnce() throws {