From caf7700e2cf1ec1ecf7b56ba5ae78f6d79b7a0ed Mon Sep 17 00:00:00 2001 From: Byeong Gwan Date: Wed, 26 Feb 2025 17:26:10 +0900 Subject: [PATCH] [iOS] Refactor error and data access (#2733) * implement safe bounded data Access and cancellation supporting URLSession downloadTask * declare specific SpineError type declaring conformance to Error on String is discouraged, and Creating own Error type is recommended * use explicit Error initializing rather than casing syntax (apply requested change) --- spine-ios/Sources/Spine/BoundsProvider.swift | 2 +- .../SkeletonDrawableWrapper+CGImage.swift | 4 +- .../Spine/SkeletonDrawableWrapper.swift | 8 +- .../Spine/Spine.Generated+Extensions.swift | 136 +++++++++++------- 4 files changed, 95 insertions(+), 55 deletions(-) diff --git a/spine-ios/Sources/Spine/BoundsProvider.swift b/spine-ios/Sources/Spine/BoundsProvider.swift index 99130336d..e2b3ca56a 100644 --- a/spine-ios/Sources/Spine/BoundsProvider.swift +++ b/spine-ios/Sources/Spine/BoundsProvider.swift @@ -59,7 +59,7 @@ public final class SkinAndAnimationBounds: NSObject, BoundsProvider { /// the bounding box of the skeleton. If no skins are given, the default skin is used. /// The `stepTime`, given in seconds, defines at what interval the bounds should be sampled /// across the entire animation. - public init(animation: String? = nil, skins: [String]? = nil, let stepTime: TimeInterval = 0.1) { + public init(animation: String? = nil, skins: [String]? = nil, stepTime: TimeInterval = 0.1) { self.animation = animation if let skins, !skins.isEmpty { self.skins = skins diff --git a/spine-ios/Sources/Spine/Extensions/SkeletonDrawableWrapper+CGImage.swift b/spine-ios/Sources/Spine/Extensions/SkeletonDrawableWrapper+CGImage.swift index 9a48e4770..ac0e6437b 100644 --- a/spine-ios/Sources/Spine/Extensions/SkeletonDrawableWrapper+CGImage.swift +++ b/spine-ios/Sources/Spine/Extensions/SkeletonDrawableWrapper+CGImage.swift @@ -27,7 +27,7 @@ public extension SkeletonDrawableWrapper { spineView.delegate?.draw(in: spineView) guard let texture = spineView.currentDrawable?.texture else { - throw "Could not read texture." + throw SpineError("Could not read texture.") } let width = texture.width let height = texture.height @@ -47,7 +47,7 @@ public extension SkeletonDrawableWrapper { let colorSpace = CGColorSpaceCreateDeviceRGB() guard let context = CGContext(data: data, width: width, height: height, bitsPerComponent: 8, bytesPerRow: rowBytes, space: colorSpace, bitmapInfo: bitmapInfo.rawValue), let cgImage = context.makeImage() else { - throw "Could not create image." + throw SpineError("Could not create image.") } return cgImage } diff --git a/spine-ios/Sources/Spine/SkeletonDrawableWrapper.swift b/spine-ios/Sources/Spine/SkeletonDrawableWrapper.swift index b87e5dd06..b5c28dbc2 100644 --- a/spine-ios/Sources/Spine/SkeletonDrawableWrapper.swift +++ b/spine-ios/Sources/Spine/SkeletonDrawableWrapper.swift @@ -94,22 +94,22 @@ public final class SkeletonDrawableWrapper: NSObject { self.skeletonData = skeletonData guard let nativeSkeletonDrawable = spine_skeleton_drawable_create(skeletonData.wrappee) else { - throw "Could not load native skeleton drawable" + throw SpineError("Could not load native skeleton drawable") } skeletonDrawable = SkeletonDrawable(nativeSkeletonDrawable) guard let nativeSkeleton = spine_skeleton_drawable_get_skeleton(skeletonDrawable.wrappee) else { - throw "Could not load native skeleton" + throw SpineError("Could not load native skeleton") } skeleton = Skeleton(nativeSkeleton) guard let nativeAnimationStateData = spine_skeleton_drawable_get_animation_state_data(skeletonDrawable.wrappee) else { - throw "Could not load native animation state data" + throw SpineError("Could not load native animation state data") } animationStateData = AnimationStateData(nativeAnimationStateData) guard let nativeAnimationState = spine_skeleton_drawable_get_animation_state(skeletonDrawable.wrappee) else { - throw "Could not load native animation state" + throw SpineError("Could not load native animation state") } animationState = AnimationState(nativeAnimationState) animationStateWrapper = AnimationStateWrapper( diff --git a/spine-ios/Sources/Spine/Spine.Generated+Extensions.swift b/spine-ios/Sources/Spine/Spine.Generated+Extensions.swift index 64b222673..5f616d2c5 100644 --- a/spine-ios/Sources/Spine/Spine.Generated+Extensions.swift +++ b/spine-ios/Sources/Spine/Spine.Generated+Extensions.swift @@ -57,18 +57,19 @@ public extension Atlas { } private static func fromData(data: Data, loadFile: (_ name: String) async throws -> Data) async throws -> (Atlas, [UIImage]) { - guard let atlasData = String(data: data, encoding: .utf8) as? NSString else { - throw "Couldn't read atlas bytes as utf8 string" + guard let atlasData = String(data: data, encoding: .utf8) else { + throw SpineError("Couldn't read atlas bytes as utf8 string") } - let atlasDataNative = UnsafeMutablePointer(mutating: atlasData.utf8String) - guard let atlas = spine_atlas_load(atlasDataNative) else { - throw "Couldn't load atlas data" + let atlas = try atlasData.utf8CString.withUnsafeBufferPointer { + guard let atlas = spine_atlas_load($0.baseAddress) else { + throw SpineError("Couldn't load atlas data") + } + return atlas } - if let error = spine_atlas_get_error(atlas) { let message = String(cString: error) spine_atlas_dispose(atlas) - throw "Couldn't load atlas: \(message)" + throw SpineError("Couldn't load atlas: \(message)") } var atlasPages = [UIImage]() @@ -130,26 +131,31 @@ public extension SkeletonData { /// /// Throws an `Error` in case the skeleton data could not be loaded. static func fromData(atlas: Atlas, data: Data) throws -> SkeletonData { - let binaryNative = try data.withUnsafeBytes { unsafeBytes in - guard let bytes = unsafeBytes.bindMemory(to: UInt8.self).baseAddress else { - throw "Couldn't read atlas binary" + let result = try data.withUnsafeBytes{ + try $0.withMemoryRebound(to: UInt8.self) { buffer in + guard let ptr = buffer.baseAddress else { + throw SpineError("Couldn't read atlas binary") + } + return spine_skeleton_data_load_binary( + atlas.wrappee, + ptr, + Int32(buffer.count) + ) } - return (data: bytes, length: Int32(unsafeBytes.count)) } - let result = spine_skeleton_data_load_binary( - atlas.wrappee, - binaryNative.data, - binaryNative.length - ) + guard let result else { + throw SpineError("Couldn't load skeleton data") + } + defer { + spine_skeleton_data_result_dispose(result) + } if let error = spine_skeleton_data_result_get_error(result) { let message = String(cString: error) - spine_skeleton_data_result_dispose(result) - throw "Couldn't load skeleton data: \(message)" + throw SpineError("Couldn't load skeleton data: \(message)") } guard let data = spine_skeleton_data_result_get_data(result) else { - throw "Couldn't load skeleton data from result" + throw SpineError("Couldn't load skeleton data from result") } - spine_skeleton_data_result_dispose(result) return SkeletonData(data) } @@ -158,26 +164,31 @@ public extension SkeletonData { /// /// Throws an `Error` in case the atlas could not be loaded. static func fromJson(atlas: Atlas, json: String) throws -> SkeletonData { - let jsonNative = UnsafeMutablePointer(mutating: (json as NSString).utf8String) - guard let result = spine_skeleton_data_load_json(atlas.wrappee, jsonNative) else { - throw "Couldn't load skeleton data json" + let result = try json.utf8CString.withUnsafeBufferPointer { buffer in + guard + let basePtr = buffer.baseAddress, + let result = spine_skeleton_data_load_json(atlas.wrappee, basePtr) else { + throw SpineError("Couldn't load skeleton data json") + } + return result + } + defer { + spine_skeleton_data_result_dispose(result) } if let error = spine_skeleton_data_result_get_error(result) { let message = String(cString: error) - spine_skeleton_data_result_dispose(result) - throw "Couldn't load skeleton data: \(message)" + throw SpineError("Couldn't load skeleton data: \(message)") } guard let data = spine_skeleton_data_result_get_data(result) else { - throw "Couldn't load skeleton data from result" + throw SpineError("Couldn't load skeleton data from result") } - spine_skeleton_data_result_dispose(result) return SkeletonData(data) } private static func fromData(atlas: Atlas, data: Data, isJson: Bool) throws -> SkeletonData { if isJson { guard let json = String(data: data, encoding: .utf8) else { - throw "Couldn't read skeleton data json string" + throw SpineError("Couldn't read skeleton data json string") } return try fromJson(atlas: atlas, json: json) } else { @@ -271,12 +282,12 @@ internal enum FileSource { case .bundle(let fileName, let bundle): let components = fileName.split(separator: ".") guard components.count > 1, let ext = components.last else { - throw "Provide both file name and file extension" + throw SpineError("Provide both file name and file extension") } let name = components.dropLast(1).joined(separator: ".") guard let fileUrl = bundle.url(forResource: name, withExtension: String(ext)) else { - throw "Could not load file with name \(name) from bundle" + throw SpineError("Could not load file with name \(name) from bundle") } return try Data(contentsOf: fileUrl, options: []) case .file(let fileUrl): @@ -289,34 +300,63 @@ internal enum FileSource { } return try Data(contentsOf: temp, options: []) } else { - return try await withCheckedThrowingContinuation { continuation in - let task = URLSession.shared.downloadTask(with: url) { temp, response, error in - if let error { - continuation.resume(throwing: error) - } else { - guard let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode == 200 else { - continuation.resume(throwing: URLError(.badServerResponse)) - return - } - guard let temp else { - continuation.resume(throwing: "Could not download file.") - return - } - do { - continuation.resume(returning: try Data(contentsOf: temp, options: [])) - } catch { + let lock = NSRecursiveLock() + nonisolated(unsafe) + var isCancelled = false + nonisolated(unsafe) + var taskHolder:URLSessionDownloadTask? = nil + return try await withTaskCancellationHandler { + try await withCheckedThrowingContinuation { continuation in + let task = URLSession.shared.downloadTask(with: url) { temp, response, error in + if let error { continuation.resume(throwing: error) + } else { + guard let httpResponse = response as? HTTPURLResponse, httpResponse.statusCode == 200 else { + continuation.resume(throwing: URLError(.badServerResponse)) + return + } + guard let temp else { + continuation.resume(throwing: SpineError("Could not download file.")) + return + } + do { + continuation.resume(returning: try Data(contentsOf: temp, options: [])) + } catch { + continuation.resume(throwing: error) + } } } + task.resume() + let shouldCancel = lock.withLock { + if !isCancelled { + taskHolder = task + } + return isCancelled + } + if shouldCancel { + task.cancel() + } } - task.resume() + } onCancel: { + lock.withLock { + isCancelled = true + let value = taskHolder + taskHolder = nil + return value + }?.cancel() } } } } } -extension String: Error { +public struct SpineError: Error, CustomStringConvertible { + + public let description: String + + internal init(_ description: String) { + self.description = description + } }