Fix various crashes and memory leaks (#520)

readium · Dec 11, 2024 · 4c860b0 · 4c860b0
1 parent f23a2e5
commit 4c860b0
Show file tree

Hide file tree

Showing 16 changed files with 79 additions and 13,237 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -6,6 +6,8 @@ All notable changes to this project will be documented in this file. Take a look
 
 ## [Unreleased]
 
+* The Readium Swift toolkit now requires a minimum of iOS 13.4.
+
 ### Changed
 
 #### Shared

diff --git a/Package.swift b/Package.swift
@@ -10,7 +10,7 @@ import PackageDescription
 let package = Package(
     name: "Readium",
     defaultLocalization: "en",
-    platforms: [.iOS(.v13)],
+    platforms: [.iOS("13.4")],
     products: [
         .library(name: "ReadiumShared", targets: ["ReadiumShared"]),
         .library(name: "ReadiumStreamer", targets: ["ReadiumStreamer"]),

diff --git a/README.md b/README.md
@@ -18,8 +18,8 @@ This toolkit is a modular project, which follows the [Readium Architecture](http
 
 | Readium   | iOS  | Swift compiler | Xcode |
 |-----------|------|----------------|-------|
-| `develop` | 13.0 | 5.10           | 15.4  |
-| 3.0.0     | 13.0 | 5.10           | 15.4  |
+| `develop` | 13.4 | 5.10           | 15.4  |
+| 3.0.0     | 13.4 | 5.10           | 15.4  |
 | 2.5.1     | 11.0 | 5.6.1          | 13.4  |
 | 2.5.0     | 10.0 | 5.6.1          | 13.4  |
 | 2.4.0     | 10.0 | 5.3.2          | 12.4  |

diff --git a/Sources/Shared/Publication/Locator.swift b/Sources/Shared/Publication/Locator.swift
@@ -300,7 +300,11 @@ public struct Locator: Hashable, CustomStringConvertible, Loggable, Sendable {
                 let highlight = highlight,
                 !highlight.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty
             else {
-                preconditionFailure("highlight is nil")
+                return Locator.Text(
+                    after: after.takeIf { !$0.isEmpty },
+                    before: before.takeIf { !$0.isEmpty },
+                    highlight: nil
+                )
             }
 
             let range = range

diff --git a/Sources/Shared/Toolkit/File/FileResource.swift b/Sources/Shared/Toolkit/File/FileResource.swift
@@ -53,16 +53,24 @@ public actor FileResource: Resource, Loggable {
     }
 
     public func stream(range: Range<UInt64>?, consume: @escaping (Data) -> Void) async -> ReadResult<Void> {
-        await handle().map { handle in
-            if let range = range {
-                handle.seek(toFileOffset: UInt64(max(0, range.lowerBound)))
-                consume(handle.readData(ofLength: Int(range.upperBound - range.lowerBound)))
-            } else {
-                handle.seek(toFileOffset: 0)
-                consume(handle.readDataToEndOfFile())
+        await handle().flatMap { handle in
+            do {
+                if let range = range {
+                    try handle.seek(toOffset: UInt64(max(0, range.lowerBound)))
+                    if let data = try handle.read(upToCount: Int(range.upperBound - range.lowerBound)) {
+                        consume(data)
+                    }
+                } else {
+                    try handle.seek(toOffset: 0)
+                    if let data = try handle.readToEnd() {
+                        consume(data)
+                    }
+                }
+            } catch {
+                return .failure(.access(.fileSystem(.io(error))))
             }
 
-            return ()
+            return .success(())
         }
     }
 

diff --git a/Sources/Shared/Toolkit/HTTP/DefaultHTTPClient.swift b/Sources/Shared/Toolkit/HTTP/DefaultHTTPClient.swift
@@ -253,27 +253,31 @@ public final class DefaultHTTPClient: HTTPClient, Loggable {
 
         func start(
             request: HTTPRequest,
-            task: URLSessionDataTask,
+            task sessionTask: URLSessionDataTask,
             receiveResponse: @escaping HTTPTask.ReceiveResponse,
             receiveChallenge: @escaping HTTPTask.ReceiveChallenge,
             consume: @escaping HTTPTask.Consume
         ) async -> HTTPResult<HTTPResponse> {
             let task = HTTPTask(
                 request: request,
-                task: task,
+                task: sessionTask,
                 receiveResponse: receiveResponse,
                 receiveChallenge: receiveChallenge,
                 consume: consume
             )
             $tasks.write { $0.append(task) }
 
-            return await withTaskCancellationHandler {
+            let result = await withTaskCancellationHandler {
                 await withCheckedContinuation { continuation in
                     task.start(with: continuation)
                 }
             } onCancel: {
                 task.cancel()
             }
+
+            $tasks.write { $0.removeAll { $0.task == sessionTask } }
+
+            return result
         }
 
         private func findTask(for urlTask: URLSessionTask) -> HTTPTask? {
@@ -373,6 +377,10 @@ public final class DefaultHTTPClient: HTTPClient, Loggable {
             self.consume = consume
         }
 
+        deinit {
+            finish()
+        }
+
         func start(with continuation: Continuation) {
             log(.info, request)
             state = .start(continuation: continuation)
@@ -385,8 +393,8 @@ public final class DefaultHTTPClient: HTTPClient, Loggable {
 
         private func finish() {
             switch state {
-            case .initializing, .start:
-                preconditionFailure("finish() called in `start` or `initializing` state")
+            case let .start(continuation):
+                continuation.resume(returning: .failure(HTTPError(kind: .cancelled)))
 
             case let .stream(continuation, response, _):
                 continuation.resume(returning: .success(response))
@@ -396,7 +404,7 @@ public final class DefaultHTTPClient: HTTPClient, Loggable {
                 log(.error, "\(request.method) \(request.url) failed with: \(error.localizedDescription)")
                 continuation.resume(returning: .failure(error))
 
-            case .finished:
+            case .initializing, .finished:
                 break
             }