Merge pull request #65 from iKenndac-forks/time-storage-fix

Add forcedCopy() method to `Region` and `TimePeriod`

.github/workflows/tests.yml

@@ -24,7 +24,7 @@ jobs:
     - name: Debug Build
       run: swift build -v -c debug
     - name: Debug Test
-      run: swift test -v -c debug
+      run: swift test -v -c debug --enable-test-discovery
 
   macos:
     name: MacOS
@@ -37,4 +37,4 @@ jobs:
     - name: Debug Build
       run: swift build -v -c debug
     - name: Debug Test
-      run: swift test -v -c debug
+      run: swift test -v -c debug --enable-test-discovery

Sources/Time/2-Calendar Core/Region.swift

@@ -43,6 +43,19 @@ public struct Region: Hashable {
         self.timeZone = timeZone
         self.locale = locale
     }
+
+    /// Create a "deep" copy of the receiver. This is a reasonably expensive operation, and should be used with care.
+    ///
+    /// This method is useful if you're on a platform that doesn't provide thread safety for the underlying date
+    /// primatives, most notably Linux at the time of writing (mid-2023). If you're using `Region` objects in a
+    /// multithreaded environment and are seeing odd behaviour, you may need to work with copies.
+    ///
+    /// For more detail, see the discussion on `TimePeriod.forcedCopy()`.
+    public func forcedCopy() -> Self {
+        return Self(calendar: Calendar(identifier: calendar.identifier),
+                    timeZone: TimeZone(identifier: timeZone.identifier) ?? TimeZone(secondsFromGMT: timeZone.secondsFromGMT()) ?? timeZone,
+                    locale: Locale(identifier: locale.identifier))
+    }
 
     /// Indicates whether time values in this region will be formatted using 12-hour ("1:00 PM") or 24-hour ("13:00") time.
     public var wants24HourTime: Bool {

Sources/Time/4-TimePeriod/TimePeriod+Initializers.swift

@@ -28,6 +28,37 @@ extension TimePeriod {
         }
     }
 
+    /// Create a "deep" copy of the receiver. This is a reasonably expensive operation, and should be used with care.
+    ///
+    /// This method is useful if you're on a platform that doesn't provide thread safety for the underlying date
+    /// primatives, most notably Linux at the time of writing (mid-2023). If you're using `TimePeriod` objects in a
+    /// multithreaded environment and are seeing odd behaviour, you may need to work with copies.
+    ///
+    /// Notable observed "odd behaviours" include:
+    ///
+    /// - Attempting to create what should be a valid `TimePeriod` range (like `someDay..<someDay.adding(days: 7)`)
+    ///   crashing with `Fatal error: Range requires lowerBound <= upperBound`.
+    ///
+    /// - Attempts to work with `AbsoluteTimePeriodSequence` or other basic time period manipulations crashing with a
+    ///   stack trace deep into Foundation's internals (often mentioning `NSCalendar`).
+    ///
+    /// For some technical background, `Calendar`/`NSCalendar` is *not* thread-safe in the `swift-corelibs-foundation`
+    /// package, which is used to provide Foundation to Swift on non-Apple platforms. Many operations that don't
+    /// outwardly appear to be mutating do temporarily perform mutating operations internally when performing
+    /// calendrical calculations, which in turn makes many nonmutating operations on `TimePeriod` not thread-safe.
+    ///
+    /// Note that modern Apple platforms (iOS, macOS, etc) have thread-safe `Calendar`/`NSCalendar` implementations
+    /// and don't suffer from this particular problem.
+    public func forcedCopy() -> Self {
+        switch storage {
+            case .absolute(let date):
+                return Self(region: region.forcedCopy(), absolute: Date(timeIntervalSince1970: date.timeIntervalSince1970))
+            case .relative(let components):
+                // Since we'll already have validated our date components, the try! here *should* be safe.
+                return try! Self(region: region.forcedCopy(), relative: components)
+        }
+    }
+
 }
 
 // Absolute initializers

Tests/LinuxMain.swift

@@ -1,6 +1,7 @@
 import XCTest
 @testable import TimeTests
 
+#if swift(<5.1)
 XCTMain([
     testCase(AbsoluteFormattingTests.allTests),
     testCase(AbsoluteTests.allTests),
@@ -11,3 +12,4 @@ XCTMain([
     testCase(TimeTests.allTests),
     testCase(SerializationTests.allTests)
 ])
+#endif

Tests/TimeTests/ThreadingTests.swift

@@ -0,0 +1,51 @@
+import Foundation
+import XCTest
+@testable import Time
+
+class ThreadingTests: XCTestCase {
+
+    static var allTests = [
+        ("testMultithreadingWithCopies", testMultithreadingWithCopies),
+    ]
+
+#if swift(>=5.5) // Concurrency was added in Swift 5.5.
+
+    func testMultithreadingWithCopies() async throws {
+        // `Calendar`/`NSCalendar` aren't thread-safe on Linux, and many `TimePeriod` operations that don't
+        // appear to be mutating do end up calling calendar methods that perform temporary mutations internally.
+        // A `forceCopy()` method was added to `Region` and `TimePeriod` to allow users of this library to create
+        // thread-local copies.
+
+        let region = Region(calendar: Calendar(identifier: .gregorian),
+                            timeZone: TimeZone(identifier: "Europe/Paris")!,
+                            locale: Locale(identifier: "en_US"))
+
+        let rangeStart = try Absolute<Minute>(region: region, year: 2023, month: 06, day: 26, hour: 14, minute: 00)
+
+        let results = await withTaskGroup(of: Range<Absolute<Minute>>.self, body: { group in
+            for _ in 0..<1000 {
+                let taskLocalStart = rangeStart.forcedCopy()
+                // ^ Without this copy, this test is likely to crash on Linux.
+                group.addTask {
+                    let range = taskLocalStart..<taskLocalStart.adding(hours: 4)
+                    XCTAssert(range.lowerBound <= range.upperBound)
+                    // ^ This assert is technially redundant since `Range` will crash if that assertion would fail.
+                    return range
+                }
+            }
+
+            var ranges = [Range<Absolute<Minute>>]()
+            for await result in group { ranges.append(result) }
+            return ranges
+        })
+
+        XCTAssertEqual(results.count, 1000)
+    }
+#else
+
+    func testMultithreadingWithCopies() throws {
+        print("WARNING: Skipping testMultithreadingWithCopies() since it requires Swift >= 5.5.")
+    }
+#endif
+
+}