Fix crash with currentLocation and data race in DefaultHTTPClient

Sources/Navigator/EPUB/EPUBNavigatorViewController.swift

@@ -504,7 +504,7 @@ open class EPUBNavigatorViewController: UIViewController, VisualNavigator, Selec
 
         let link = spreadView.focusedResource ?? spreadView.spread.leading
         let href = link.href
-        let progression = spreadView.progression(in: href)
+        let progression = min(max(spreadView.progression(in: href), 0.0), 1.0)
 
         // The positions are not always available, for example a Readium WebPub doesn't have any
         // unless a Publication Positions Web Service is provided.

Sources/Navigator/EPUB/EPUBReflowableSpreadView.swift

@@ -321,9 +321,11 @@ final class EPUBReflowableSpreadView: EPUBSpreadView {
 
     // Called by the javascript code to notify that scrolling ended.
     private func progressionDidChange(_ body: Any) {
-        guard spreadLoaded, let bodyString = body as? String, let newProgression = Double(bodyString) else {
+        guard spreadLoaded, let bodyString = body as? String, var newProgression = Double(bodyString) else {
             return
         }
+        newProgression = min(max(newProgression, 0.0), 1.0)
+
         if previousProgression == nil {
             previousProgression = progression
         }

Sources/Shared/Toolkit/Atomic.swift

@@ -0,0 +1,76 @@
+//
+//  Copyright 2022 Readium Foundation. All rights reserved.
+//  Use of this source code is governed by the BSD-style license
+//  available in the top-level LICENSE file of the project.
+//
+
+import Foundation
+
+/// Smart pointer protecting concurrent access to its memory to avoid data races.
+///
+/// This is also a property wrapper, which makes it easy to use as:
+/// ```
+/// @Atomic var data: Int
+/// ```
+///
+/// The property becomes read-only, to prevent a common error when modifying the property using its
+/// previous value. For example:
+/// ```
+/// data += 1
+/// ```
+/// This is not safe, because it's actually two operations: a read and a write. The value might have changed
+/// between the moment you read it and when you write the result of incrementing the value.
+///
+/// Instead, you must use `write()` to mutate the property:
+/// ```
+/// $data.write { value in
+///     value += 1
+/// }
+/// ```
+@propertyWrapper
+public final class Atomic<Value> {
+    private var value: Value
+
+    /// Queue used to protect accesses to `value`.
+    ///
+    /// We could use a serial queue but that would impact performances as concurrent reads would not be
+    /// possible. To make sure we don't get data races, writes are done using a `.barrier` flag.
+    private let queue = DispatchQueue(label: "org.readium.Atomic", attributes: .concurrent)
+
+    public init(wrappedValue value: Value) {
+        self.value = value
+    }
+
+    public var wrappedValue: Value {
+        get { read() }
+        set { fatalError("Use $property.write { $0 = ... } to mutate this property") }
+    }
+
+    public var projectedValue: Atomic<Value> {
+        return self
+    }
+
+    /// Reads the current value synchronously.
+    public func read() -> Value {
+        queue.sync {
+            value
+        }
+    }
+
+    /// Reads the current value asynchronously.
+    public func read(completion: @escaping (Value) -> Void) {
+        queue.async {
+            completion(self.value)
+        }
+    }
+
+    /// Writes the value synchronously in a safe way.
+    public func write(_ changes: (inout Value) -> Void) {
+        // The `barrier` flag here guarantees that we will never have a
+        // concurrent read on `value` while we are modifying it. This prevents
+        // a data race.
+        queue.sync(flags: .barrier) {
+            changes(&value)
+        }
+    }
+}

Sources/Shared/Toolkit/HTTP/DefaultHTTPClient.swift

@@ -252,45 +252,39 @@ public final class DefaultHTTPClient: NSObject, HTTPClient, Loggable, URLSession
     // MARK: - Task Management
 
     /// On-going tasks.
-    private var tasks: [Task] = []
-
-    private func findTaskIndex(_ task: URLSessionTask) -> Int? {
-        let i = tasks.firstIndex(where: { $0.task == task})
-        if i == nil {
-            log(.error, "Cannot find on-going HTTP task for \(task)")
-        }
-        return i
-    }
-
+    @Atomic private var tasks: [Task] = []
+
     private func start(_ task: Task) -> Cancellable {
-        tasks.append(task)
+        $tasks.write { $0.append(task) }
         task.start()
         return task
     }
 
+    private func findTask(for urlTask: URLSessionTask) -> Task? {
+        let task = tasks.first { $0.task == urlTask}
+        if task == nil {
+            log(.error, "Cannot find on-going HTTP task for \(urlTask)")
+        }
+        return task
+    }
+
 
     // MARK: - URLSessionDataDelegate
 
     public func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive response: URLResponse, completionHandler: @escaping (URLSession.ResponseDisposition) -> ()) {
-        guard let i = findTaskIndex(dataTask) else {
+        guard let task = findTask(for: dataTask) else {
             completionHandler(.cancel)
             return
         }
-        tasks[i].urlSession(session, didReceive: response, completionHandler: completionHandler)
+        task.urlSession(session, didReceive: response, completionHandler: completionHandler)
     }
 
     public func urlSession(_ session: URLSession, dataTask: URLSessionDataTask, didReceive data: Data) {
-        guard let i = findTaskIndex(dataTask) else {
-            return
-        }
-        tasks[i].urlSession(session, didReceive: data)
+        findTask(for: dataTask)?.urlSession(session, didReceive: data)
     }
 
     public func urlSession(_ session: URLSession, task: URLSessionTask, didCompleteWithError error: Error?) {
-        guard let i = findTaskIndex(task) else {
-            return
-        }
-        tasks[i].urlSession(session, didCompleteWithError: error)
+        findTask(for: task)?.urlSession(session, didCompleteWithError: error)
     }
 
 

Support/Carthage/.xcodegen

@@ -1,5 +1,5 @@
 # XCODEGEN VERSION
-2.25.0
+2.29.0
 
 # SPEC
 {
@@ -7504,6 +7504,7 @@
 ../../Sources/Shared/Publication/Services/Locator/DefaultLocatorService.swift
 ../../Sources/Shared/Publication/Services/Locator/LocatorService.swift
 ../../Sources/Shared/Publication/Services/Positions
+../../Sources/Shared/Publication/Services/Positions/InMemoryPositionsService.swift
 ../../Sources/Shared/Publication/Services/Positions/PerResourcePositionsService.swift
 ../../Sources/Shared/Publication/Services/Positions/PositionsService.swift
 ../../Sources/Shared/Publication/Services/PublicationService.swift
@@ -7525,6 +7526,7 @@
 ../../Sources/Shared/Toolkit/Archive/ExplodedArchive.swift
 ../../Sources/Shared/Toolkit/Archive/Minizip.swift
 ../../Sources/Shared/Toolkit/Archive/ZIPFoundation.swift
+../../Sources/Shared/Toolkit/Atomic.swift
 ../../Sources/Shared/Toolkit/Cancellable.swift
 ../../Sources/Shared/Toolkit/CancellableResult.swift
 ../../Sources/Shared/Toolkit/ControlFlow.swift

Support/Carthage/Readium.xcodeproj/project.pbxproj

@@ -157,6 +157,7 @@
 		825642E013351C922B6510AD /* UTI.swift in Sources */ = {isa = PBXBuildFile; fileRef = 48B28C65845F0575C40877F6 /* UTI.swift */; };
 		82BAA3EB081DD29A928958AC /* ContentLayout.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4A496C959F870BAFDB447DA /* ContentLayout.swift */; };
 		837C0BC3151E302508B4BC44 /* LCPAuthenticating.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2CB0BFECA8236412881393AA /* LCPAuthenticating.swift */; };
+		861C71906603180ABD01E8FA /* Atomic.swift in Sources */ = {isa = PBXBuildFile; fileRef = CBB57FCAEE605484A7290DBB /* Atomic.swift */; };
 		862098954DE3522E87E806CC /* Minizip.xcframework in Frameworks */ = {isa = PBXBuildFile; fileRef = CFFEBDFE931745C07DACD4A3 /* Minizip.xcframework */; };
 		874BD412CBA1D392451B952B /* Assets in Resources */ = {isa = PBXBuildFile; fileRef = 251275D0DF87F85158A5FEA9 /* Assets */; };
 		88A171A36700ACF5A4AD6305 /* PublicationService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 667B76C4766DFF58D066D40B /* PublicationService.swift */; };
@@ -538,6 +539,7 @@
 		C5E7CEDF6EA681FE8119791B /* Feed.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Feed.swift; sourceTree = "<group>"; };
 		C7931CB2A5658CAAECD150B0 /* NSRegularExpression.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NSRegularExpression.swift; sourceTree = "<group>"; };
 		CAD79372361D085CA0500CF4 /* Properties+OPDS.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Properties+OPDS.swift"; sourceTree = "<group>"; };
+		CBB57FCAEE605484A7290DBB /* Atomic.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Atomic.swift; sourceTree = "<group>"; };
 		CC925E451D875E5F74748EDC /* Optional.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Optional.swift; sourceTree = "<group>"; };
 		CDA8111A330AB4D7187DD743 /* LocatorService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LocatorService.swift; sourceTree = "<group>"; };
 		CE641F78FD99A426A80B3495 /* Zip.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Zip.h; sourceTree = "<group>"; };
@@ -1287,6 +1289,7 @@
 		C42B511253C3D9C6DA8AA5CC /* Toolkit */ = {
 			isa = PBXGroup;
 			children = (
+				CBB57FCAEE605484A7290DBB /* Atomic.swift */,
 				7BBD54FD376456C1925316BC /* Cancellable.swift */,
 				0CB0D3EE83AE0CE1F0B0B0CF /* CancellableResult.swift */,
 				55BC4119B8937D17ED80B1AB /* ControlFlow.swift */,
@@ -1687,7 +1690,7 @@
 				};
 			};
 			buildConfigurationList = 5A872BCD95ECE5673BC89051 /* Build configuration list for PBXProject "Readium" */;
-			compatibilityVersion = "Xcode 10.0";
+			compatibilityVersion = "Xcode 11.0";
 			developmentRegion = en;
 			hasScannedForEncodings = 0;
 			knownRegions = (
@@ -1842,6 +1845,7 @@
 				09B7475BC8E63C940BD5881A /* Archive.swift in Sources */,
 				A3EBB38968F8EB4ABC560678 /* ArchiveFetcher.swift in Sources */,
 				AA218336FBD1C23959542515 /* Array.swift in Sources */,
+				861C71906603180ABD01E8FA /* Atomic.swift in Sources */,
 				5C8ED4151A6C7EF6608A03F8 /* AudioSession.swift in Sources */,
 				8E4C9F5A53A6F9B8FC28B7D4 /* BufferedResource.swift in Sources */,
 				6D27F5B8C7DBFBF5FB99A4BE /* Bundle.swift in Sources */,

Support/Carthage/Readium.xcodeproj/xcshareddata/xcschemes/R2Navigator.xcscheme

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
    LastUpgradeVersion = "1250"
-   version = "1.3">
+   version = "1.7">
    <BuildAction
       parallelizeBuildables = "YES"
       buildImplicitDependencies = "YES"

Support/Carthage/Readium.xcodeproj/xcshareddata/xcschemes/R2Shared.xcscheme

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
    LastUpgradeVersion = "1250"
-   version = "1.3">
+   version = "1.7">
    <BuildAction
       parallelizeBuildables = "YES"
       buildImplicitDependencies = "YES"

Support/Carthage/Readium.xcodeproj/xcshareddata/xcschemes/R2Streamer.xcscheme

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
    LastUpgradeVersion = "1250"
-   version = "1.3">
+   version = "1.7">
    <BuildAction
       parallelizeBuildables = "YES"
       buildImplicitDependencies = "YES"

Support/Carthage/Readium.xcodeproj/xcshareddata/xcschemes/ReadiumLCP.xcscheme

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
    LastUpgradeVersion = "1250"
-   version = "1.3">
+   version = "1.7">
    <BuildAction
       parallelizeBuildables = "YES"
       buildImplicitDependencies = "YES"

Support/Carthage/Readium.xcodeproj/xcshareddata/xcschemes/ReadiumOPDS.xcscheme

@@ -1,7 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <Scheme
    LastUpgradeVersion = "1250"
-   version = "1.3">
+   version = "1.7">
    <BuildAction
       parallelizeBuildables = "YES"
       buildImplicitDependencies = "YES"