diff --git a/Sources/PlaydateKit/File/Classes/Handle.swift b/Sources/PlaydateKit/File/Classes/Handle.swift index 15115bb..5793a6d 100644 --- a/Sources/PlaydateKit/File/Classes/Handle.swift +++ b/Sources/PlaydateKit/File/Classes/Handle.swift @@ -1,11 +1,13 @@ internal import CPlaydate extension File { - /// An open file. Wraps `SDFile`. The file is closed on deinit if it has - /// not been closed explicitly. - public final class Handle { + /// An open file. Wraps `SDFile`. The file is closed when the handle goes + /// out of scope, unless it was closed explicitly with `close()`. + /// + /// The handle is non-copyable: it has a single owner, so it cannot be + /// used after `close()` and no heap allocation backs it. + public struct Handle: ~Copyable { let pointer: UnsafeMutableRawPointer - private var isClosed = false /// Opens the file at `path`. public init(path: String, mode: Options) throws(PlaydateError) { @@ -17,15 +19,16 @@ extension File { } deinit { - if !isClosed { - _ = fileAPI.pointee.close.unsafelyUnwrapped(pointer) - } + _ = fileAPI.pointee.close.unsafelyUnwrapped(pointer) } - /// Closes the file. Further operations are invalid. - public func close() throws(PlaydateError) { - guard !isClosed else { return } - isClosed = true + /// Closes the file, consuming the handle. + // `@export(interface)` lets `discard` compile in Embedded Swift with + // the 6.4 release toolchain; later toolchains accept it without. + @export(interface) + public consuming func close() throws(PlaydateError) { + let pointer = self.pointer + discard self if fileAPI.pointee.close.unsafelyUnwrapped(pointer) != 0 { throw lastFileError() } } diff --git a/Sources/PlaydateKit/Graphics/Classes/StreamPlayer.swift b/Sources/PlaydateKit/Graphics/Classes/StreamPlayer.swift index 46bf11e..6d4ef25 100644 --- a/Sources/PlaydateKit/Graphics/Classes/StreamPlayer.swift +++ b/Sources/PlaydateKit/Graphics/Classes/StreamPlayer.swift @@ -8,8 +8,10 @@ extension Graphics { /// Wraps `LCDStreamPlayer`. public final class StreamPlayer { let pointer: OpaquePointer - /// Retains the active source so it outlives the stream. + /// Retains the active source so it outlives the stream. A file + /// handle is owned outright, as it cannot be shared. private var retainedSource: AnyObject? + private var retainedFile: File.Handle? public init() { pointer = streamAPI.pointee.newPlayer.unsafelyUnwrapped().unsafelyUnwrapped @@ -24,22 +26,26 @@ extension Graphics { streamAPI.pointee.setBufferSize.unsafelyUnwrapped(pointer, Int32(video), Int32(audio)) } - /// Streams from an open file. - public func setFile(_ file: File.Handle) { - retainedSource = file + /// Streams from an open file. The stream takes ownership of the + /// handle and closes it when the source changes or the stream is freed. + public func setFile(_ file: consuming File.Handle) { streamAPI.pointee.setFile.unsafelyUnwrapped(pointer, file.pointer) + retainedFile = consume file + retainedSource = nil } /// Streams from an HTTP connection. public func setHTTPConnection(_ connection: Network.HTTPConnection) { - retainedSource = connection streamAPI.pointee.setHTTPConnection.unsafelyUnwrapped(pointer, connection.pointer) + retainedSource = connection + retainedFile = nil } /// Streams from a TCP connection. public func setTCPConnection(_ connection: Network.TCPConnection) { - retainedSource = connection streamAPI.pointee.setTCPConnection.unsafelyUnwrapped(pointer, connection.pointer) + retainedSource = connection + retainedFile = nil } /// The player used for the stream's audio track. Owned by the stream. diff --git a/Sources/PlaydateKit/JSON/JSON.swift b/Sources/PlaydateKit/JSON/JSON.swift index 93241a0..f61bb22 100644 --- a/Sources/PlaydateKit/JSON/JSON.swift +++ b/Sources/PlaydateKit/JSON/JSON.swift @@ -118,27 +118,21 @@ extension JSON { } /// Decodes JSON read from an open file into a `Value` tree. - public static func decode(file: File.Handle) throws(PlaydateError) -> Value { + public static func decode(file: borrowing File.Handle) throws(PlaydateError) -> Value { let context = DecodeContext() var decoder = makeDecoder(context: Unmanaged.passUnretained(context)) var reader = json_reader() - reader.userdata = Unmanaged.passUnretained(file).toOpaque() + // The handle is borrowed for the whole call, so its `SDFile` stays + // open while the decoder reads through it. + reader.userdata = file.pointer reader.read = { userdata, buffer, size in guard let userdata, let buffer else { return -1 } - let file = Unmanaged.fromOpaque(userdata).takeUnretainedValue() - var destination = UnsafeMutableBufferPointer(start: buffer, count: Int(size)).mutableSpan - do { - let count = try file.read(into: &destination) - return count > 0 ? Int32(count) : -1 - } catch { - return -1 - } + let count = fileAPI.pointee.read.unsafelyUnwrapped(userdata, buffer, UInt32(size)) + return count > 0 ? count : -1 } var outval = json_value() let ok = withExtendedLifetime(context) { - withExtendedLifetime(file) { - jsonAPI.pointee.decode.unsafelyUnwrapped(&decoder, reader, &outval) != 0 - } + jsonAPI.pointee.decode.unsafelyUnwrapped(&decoder, reader, &outval) != 0 } guard ok else { // A completed root container may already have been written to diff --git a/Tests/PlaydateKit/MockPlaydate.swift b/Tests/PlaydateKit/MockPlaydate.swift index 1420be4..972bdae 100644 --- a/Tests/PlaydateKit/MockPlaydate.swift +++ b/Tests/PlaydateKit/MockPlaydate.swift @@ -422,6 +422,19 @@ enum Mock { private static func installJSON() { jsonAPI.initialize(to: playdate_json()) + // Pulls one chunk through the reader, as the OS would, and decodes + // it as `null`. + jsonAPI.pointee.decode = { _, reader, outval in + var buffer = [UInt8](repeating: 0, count: 16) + let count = buffer.withUnsafeMutableBufferPointer { buffer in + reader.read?(reader.userdata, buffer.baseAddress, Int32(buffer.count)) ?? -1 + } + Mock.record("decode(\(count))") + outval?.pointee = json_value() + outval?.pointee.type = CChar(kJSONNull.rawValue) + return 1 + } + jsonAPI.pointee.decodeString = { decoder, _, outval in guard let decoder else { return 0 } diff --git a/Tests/PlaydateKit/WrapperTests.swift b/Tests/PlaydateKit/WrapperTests.swift index df5cf3f..f5b7434 100644 --- a/Tests/PlaydateKit/WrapperTests.swift +++ b/Tests/PlaydateKit/WrapperTests.swift @@ -345,6 +345,26 @@ struct WrapperTests { #expect(Mock.eventCount("close") == 1) } + @Test func jsonDecodeFileReadsThroughTheHandleAndClosesIt() throws { + Mock.fileReadLimit = 5 + let value = try JSON.decodeFile(path: "save.json") + guard case .null = value else { + Issue.record("expected .null, got \(value)") + return + } + #expect(Mock.events.contains("decode(5)")) + #expect(Mock.eventCount("close") == 1) + } + + @Test func fileHandleClosesWhenItGoesOutOfScope() throws { + do { + let handle = try File.Handle(path: "save.dat", mode: .write) + _ = try handle.write([1]) + #expect(Mock.eventCount("close") == 0) + } + #expect(Mock.eventCount("close") == 1) + } + @Test func fileHandleReadLengthReturnsOnlyTheBytesRead() throws { let handle = try File.Handle(path: "save.dat", mode: [.read, .readData])