From 1d9504c2255accdb5852a424e372ce944a64df90 Mon Sep 17 00:00:00 2001 From: Javier Cicchelli Date: Sat, 25 Jul 2026 10:28:24 +0200 Subject: [PATCH] Fixed some wrapper lifetime and leak issues found in PlayDate bindings target. --- Sources/PlayDate/GraphicsVideo.swift | 11 ++++++++++- Sources/PlayDate/JSON.swift | 6 ++++++ Sources/PlayDate/Network.swift | 14 +++++++------- Sources/PlayDate/Sound.swift | 27 +++++++++++++++++++++------ Sources/PlayDate/Sprite.swift | 7 ++++++- Sources/PlayDate/Support.swift | 9 +++++---- 6 files changed, 55 insertions(+), 19 deletions(-) diff --git a/Sources/PlayDate/GraphicsVideo.swift b/Sources/PlayDate/GraphicsVideo.swift index 1f6bea5..96ab78e 100644 --- a/Sources/PlayDate/GraphicsVideo.swift +++ b/Sources/PlayDate/GraphicsVideo.swift @@ -118,11 +118,20 @@ extension Graphics { } /// The player used for the stream's audio track. Owned by the stream. + /// The same wrapper is returned on every access, so callbacks + /// registered on it stay valid for the stream's lifetime. public var filePlayer: Sound.FilePlayer? { guard let player = streamAPI.pointee.getFilePlayer.unsafelyUnwrapped(pointer) else { return nil } - return Sound.FilePlayer(pointer: player, isOwned: false) + if let cached = cachedFilePlayer, cached.pointer == player { + return cached + } + let wrapper = Sound.FilePlayer(pointer: player, isOwned: false) + cachedFilePlayer = wrapper + return wrapper } + private var cachedFilePlayer: Sound.FilePlayer? + /// The player used for the stream's video track. Owned by the stream. public var videoPlayer: VideoPlayer? { guard let player = streamAPI.pointee.getVideoPlayer.unsafelyUnwrapped(pointer) else { return nil } diff --git a/Sources/PlayDate/JSON.swift b/Sources/PlayDate/JSON.swift index 8b5ea92..a8a3e7c 100644 --- a/Sources/PlayDate/JSON.swift +++ b/Sources/PlayDate/JSON.swift @@ -116,6 +116,9 @@ extension JSON { } } guard ok else { + // A completed root container may already have been written to + // outval before the failure; consume it so its box is not leaked. + _ = convert(outval) throw decodeError(context) } return convert(outval) @@ -145,6 +148,9 @@ extension JSON { } } guard ok else { + // A completed root container may already have been written to + // outval before the failure; consume it so its box is not leaked. + _ = convert(outval) throw decodeError(context) } return convert(outval) diff --git a/Sources/PlayDate/Network.swift b/Sources/PlayDate/Network.swift index 506170d..593d855 100644 --- a/Sources/PlayDate/Network.swift +++ b/Sources/PlayDate/Network.swift @@ -70,21 +70,21 @@ extension Network { } /// Turns the wifi radio on or off. The completion receives `nil` on - /// success. + /// success. Completions of overlapping calls are delivered in call order. public static func setEnabled(_ enabled: Bool, completion: ((NetError?) -> Void)? = nil) { - setEnabledCompletion = completion - if completion != nil { + if let completion { + setEnabledCompletions.append(completion) networkAPI.pointee.setEnabled.unsafelyUnwrapped(enabled, { error in - let completion = Network.setEnabledCompletion - Network.setEnabledCompletion = nil - completion?(Network.optionalError(error)) + guard !Network.setEnabledCompletions.isEmpty else { return } + let completion = Network.setEnabledCompletions.removeFirst() + completion(Network.optionalError(error)) }) } else { networkAPI.pointee.setEnabled.unsafelyUnwrapped(enabled, nil) } } - nonisolated(unsafe) private static var setEnabledCompletion: ((NetError?) -> Void)? + nonisolated(unsafe) private static var setEnabledCompletions: [(NetError?) -> Void] = [] /// Requests permission to connect to `server`. Shared by HTTP and TCP. fileprivate static func requestAccess( diff --git a/Sources/PlayDate/Sound.swift b/Sources/PlayDate/Sound.swift index 2272005..441421b 100644 --- a/Sources/PlayDate/Sound.swift +++ b/Sources/PlayDate/Sound.swift @@ -207,11 +207,13 @@ extension Sound { } /// The default channel, which sources are added to unless otherwise - /// specified. - public static var `default`: Channel { + /// specified. A single shared wrapper, so resources retained through + /// it (sources, effects, modulators) stay alive. + public static var `default`: Channel { defaultChannel } + + nonisolated(unsafe) private static let defaultChannel = Channel(pointer: snd.pointee.getDefaultChannel.unsafelyUnwrapped().unsafelyUnwrapped, isOwned: false) - } nonisolated(unsafe) private static var addedChannels: [Channel] = [] @@ -246,8 +248,12 @@ extension Sound { @discardableResult public func removeSource(_ source: Source) -> Bool { let removed = Channel.api.pointee.removeSource.unsafelyUnwrapped(pointer, source.pointer) != 0 - retainedSources.removeAll { $0 === source } - CallbackSource.release(source) + // Only drop the retentions if the source was actually on this + // channel; otherwise another channel may still be pulling it. + if removed { + retainedSources.removeAll { $0 === source } + CallbackSource.release(source) + } return removed } @@ -322,13 +328,22 @@ extension Sound { } /// The channel's output as a source, for feeding into another channel. + /// The same wrapper is returned on every access, so callbacks + /// registered on it stay valid for the channel's lifetime. public var outputAsSource: Source? { guard let source = Channel.api.pointee.getOutputAsSource.unsafelyUnwrapped(pointer) else { return nil } - return Source(pointer: source, isOwned: false) + if let cached = cachedOutputSource, cached.pointer == source { + return cached + } + let wrapper = Source(pointer: source, isOwned: false) + cachedOutputSource = wrapper + return wrapper } + private var cachedOutputSource: Source? + private func retain(_ modulator: SignalValue?) { if let modulator, !retainedModulators.contains(where: { $0 === modulator }) { retainedModulators.append(modulator) diff --git a/Sources/PlayDate/Sprite.swift b/Sources/PlayDate/Sprite.swift index 013402b..3ab5788 100644 --- a/Sources/PlayDate/Sprite.swift +++ b/Sources/PlayDate/Sprite.swift @@ -55,7 +55,12 @@ public final class Sprite { init(pointer: OpaquePointer, isOwned: Bool) { self.pointer = pointer self.isOwned = isOwned - spriteAPI.pointee.setUserdata.unsafelyUnwrapped(pointer, Unmanaged.passUnretained(self).toOpaque()) + // Transient wrappers for sprites created outside the binding must not + // store a back-reference: it would dangle once the wrapper is + // deallocated, and only owned wrappers clear it in `deinit`. + if isOwned { + spriteAPI.pointee.setUserdata.unsafelyUnwrapped(pointer, Unmanaged.passUnretained(self).toOpaque()) + } } /// Allocates a new sprite. diff --git a/Sources/PlayDate/Support.swift b/Sources/PlayDate/Support.swift index 623f87f..2b7de37 100644 --- a/Sources/PlayDate/Support.swift +++ b/Sources/PlayDate/Support.swift @@ -71,16 +71,17 @@ extension String { #if hasFeature(Embedded) && !os(macOS) /// The Embedded Swift runtime allocates through `posix_memalign(3)`, which /// the Playdate device C library does not provide. Memory comes from -/// `malloc`, which the SDK's setup code routes to the firmware allocator; -/// that allocator hands out sufficiently aligned blocks, so alignment is -/// only asserted, not adjusted. +/// `malloc`, which the SDK's setup code routes to the firmware allocator. +/// The pointer is later released with plain `free`, so it cannot be offset +/// to adjust alignment; the firmware allocator's natural alignment has to +/// satisfy the request, which the precondition asserts. @_cdecl("posix_memalign") public func posix_memalign( _ memptr: UnsafeMutablePointer, _ alignment: Int, _ size: Int ) -> CInt { - guard let allocation = malloc(size + alignment - 1) else { fatalError() } + guard let allocation = malloc(size) else { fatalError() } precondition(Int(bitPattern: allocation) % alignment == 0) memptr.pointee = allocation return 0