Restrict the use of ThreadLocal to immortal storage only

swiftwasm · Nov 28, 2024 · d4e0ee8 · d4e0ee8
1 parent 49b207a
commit d4e0ee8
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 35 deletions.
diff --git a/Sources/JavaScriptKit/FundamentalObjects/JSObject.swift b/Sources/JavaScriptKit/FundamentalObjects/JSObject.swift
@@ -206,19 +206,10 @@ public class JSObject: Equatable {
 
     // `JSObject` storage itself is immutable, and use of `JSObject.global` from other
     // threads maintains the same semantics as `globalThis` in JavaScript.
-    #if compiler(>=6.1) && _runtime(_multithreaded)
-        @LazyThreadLocal(initialize: {
-            return JSObject(id: _JS_Predef_Value_Global)
-        })
-        private static var _global: JSObject
-    #else
-        #if compiler(>=5.10)
-        nonisolated(unsafe)
-        static let _global = JSObject(id: _JS_Predef_Value_Global)
-        #else
-        static let _global = JSObject(id: _JS_Predef_Value_Global)
-        #endif
-    #endif
+    @LazyThreadLocal(initialize: {
+        return JSObject(id: _JS_Predef_Value_Global)
+    })
+    private static var _global: JSObject
 
     deinit {
         assertOnOwnerThread(hint: "deinitializing")

diff --git a/Sources/JavaScriptKit/ThreadLocal.swift b/Sources/JavaScriptKit/ThreadLocal.swift
@@ -1,4 +1,3 @@
-#if compiler(>=6.1) && _runtime(_multithreaded)
 #if canImport(wasi_pthread)
 import wasi_pthread
 #elseif canImport(Darwin)
@@ -9,8 +8,14 @@ import Glibc
 #error("Unsupported platform")
 #endif
 
+/// A property wrapper that provides thread-local storage for a value.
+///
+/// The value is stored in a thread-local variable, which is a separate copy for each thread.
 @propertyWrapper
 final class ThreadLocal<Value>: Sendable {
+#if compiler(>=6.1) && _runtime(_multithreaded)
+    /// The wrapped value stored in the thread-local storage.
+    /// The initial value is `nil` for each thread.
     var wrappedValue: Value? {
         get {
             guard let pointer = pthread_getspecific(key) else {
@@ -34,6 +39,8 @@ final class ThreadLocal<Value>: Sendable {
     private let fromPointer: @Sendable (UnsafeMutableRawPointer) -> Value
     private let release: @Sendable (UnsafeMutableRawPointer) -> Void
 
+    /// A constructor that requires `Value` to be `AnyObject` to be
+    /// able to store the value directly in the thread-local storage.
     init() where Value: AnyObject {
         var key = pthread_key_t()
         pthread_key_create(&key, nil)
@@ -43,13 +50,15 @@ final class ThreadLocal<Value>: Sendable {
         self.release = { Unmanaged<Value>.fromOpaque($0).release() }
     }
 
-    class Box {
+    private class Box {
         let value: Value
         init(_ value: Value) {
             self.value = value
         }
     }
 
+    /// A constructor that doesn't require `Value` to be `AnyObject` but
+    /// boxing the value in heap-allocated memory.
     init(boxing _: Void) {
         var key = pthread_key_t()
         pthread_key_create(&key, nil)
@@ -65,15 +74,26 @@ final class ThreadLocal<Value>: Sendable {
         }
         self.release = { Unmanaged<Box>.fromOpaque($0).release() }
     }
+#else
+    // Fallback implementation for platforms that don't support pthread
+
+    var wrappedValue: Value?
+
+    init() where Value: AnyObject {
+        wrappedValue = nil
+    }
+    init(boxing _: Void) {
+        wrappedValue = nil
+    }
+#endif
 
     deinit {
-        if let oldPointer = pthread_getspecific(key) {
-            release(oldPointer)
-        }
-        pthread_key_delete(key)
+        preconditionFailure("ThreadLocal can only be used as an immortal storage, cannot be deallocated")
     }
 }
 
+/// A property wrapper that lazily initializes a thread-local value
+/// for each thread that accesses the value.
 @propertyWrapper
 final class LazyThreadLocal<Value>: Sendable {
     private let storage: ThreadLocal<Value>
@@ -99,5 +119,3 @@ final class LazyThreadLocal<Value>: Sendable {
         self.initialValue = initialize
     }
 }
-
-#endif
diff --git a/Tests/JavaScriptKitTests/ThreadLocalTests.swift b/Tests/JavaScriptKitTests/ThreadLocalTests.swift
@@ -10,41 +10,40 @@ final class ThreadLocalTests: XCTestCase {
     func testLeak() throws {
         struct Check {
             @ThreadLocal
-            var value: MyHeapObject?
+            static var value: MyHeapObject?
             @ThreadLocal(boxing: ())
-            var value2: MyStruct?
+            static var value2: MyStruct?
         }
         weak var weakObject: MyHeapObject?
         do {
             let object = MyHeapObject()
             weakObject = object
-            let check = Check()
-            check.value = object
-            XCTAssertNotNil(check.value)
-            XCTAssertTrue(check.value === object)
+            Check.value = object
+            XCTAssertNotNil(Check.value)
+            XCTAssertTrue(Check.value === object)
+            Check.value = nil
         }
         XCTAssertNil(weakObject)
 
         weak var weakObject2: MyHeapObject?
         do {
             let object = MyHeapObject()
             weakObject2 = object
-            let check = Check()
-            check.value2 = MyStruct(object: object)
-            XCTAssertNotNil(check.value2)
-            XCTAssertTrue(check.value2!.object === object)
+            Check.value2 = MyStruct(object: object)
+            XCTAssertNotNil(Check.value2)
+            XCTAssertTrue(Check.value2!.object === object)
+            Check.value2 = nil
         }
         XCTAssertNil(weakObject2)
     }
 
     func testLazyThreadLocal() throws {
         struct Check {
             @LazyThreadLocal(initialize: { MyHeapObject() })
-            var value: MyHeapObject
+            static var value: MyHeapObject
         }
-        let check = Check()
-        let object1 = check.value
-        let object2 = check.value
+        let object1 = Check.value
+        let object2 = Check.value
         XCTAssertTrue(object1 === object2)
     }
 }