Merge pull request #274 from swiftwasm/yt/fix-multithread-cache-issue

Stop use of global variable as a object cache

Author: kateinoigakukun

Diff for: Sources/JavaScriptEventLoop/WebWorkerTaskExecutor.swift

@@ -200,6 +200,7 @@ public final class WebWorkerTaskExecutor: TaskExecutor {
             parentTaskExecutor = executor
             // Store the thread ID to the worker. This notifies the main thread that the worker is started.
             self.tid.store(tid, ordering: .sequentiallyConsistent)
+            trace("Worker.start tid=\(tid)")
         }
 
         /// Process jobs in the queue.
@@ -212,7 +213,14 @@ public final class WebWorkerTaskExecutor: TaskExecutor {
             guard let executor = parentTaskExecutor else {
                 preconditionFailure("The worker must be started with a parent executor.")
             }
-            assert(state.load(ordering: .sequentiallyConsistent) == .running, "Invalid state: not running")
+            do {
+                // Assert the state at the beginning of the run.
+                let state = state.load(ordering: .sequentiallyConsistent)
+                assert(
+                    state == .running || state == .terminated,
+                    "Invalid state: not running (tid=\(self.tid.load(ordering: .sequentiallyConsistent)), \(state))"
+                )
+            }
             while true {
                 // Pop a job from the queue.
                 let job = jobQueue.withLock { queue -> UnownedJob? in
@@ -247,7 +255,7 @@ public final class WebWorkerTaskExecutor: TaskExecutor {
 
         /// Terminate the worker.
         func terminate() {
-            trace("Worker.terminate")
+            trace("Worker.terminate tid=\(tid.load(ordering: .sequentiallyConsistent))")
             state.store(.terminated, ordering: .sequentiallyConsistent)
             let tid = self.tid.load(ordering: .sequentiallyConsistent)
             guard tid != 0 else {
@@ -283,6 +291,7 @@ public final class WebWorkerTaskExecutor: TaskExecutor {
                     self.worker = worker
                 }
             }
+            trace("Executor.start")
             // Start worker threads via pthread_create.
             for worker in workers {
                 // NOTE: The context must be allocated on the heap because

Diff for: Sources/JavaScriptKit/BasicObjects/JSArray.swift

@@ -93,9 +93,9 @@ extension JSArray: RandomAccessCollection {
     }
 }
 
-private let alwaysTrue = JSClosure { _ in .boolean(true) }
+private let alwaysTrue = LazyThreadLocal(initialize: { JSClosure { _ in .boolean(true) } })
 private func getObjectValuesLength(_ object: JSObject) -> Int {
-    let values = object.filter!(alwaysTrue).object!
+    let values = object.filter!(alwaysTrue.wrappedValue).object!
     return Int(values.length.number!)
 }
 

Diff for: Sources/JavaScriptKit/ConvertibleToJSValue.swift

@@ -85,8 +85,10 @@ extension JSObject: JSValueCompatible {
     // from `JSFunction`
 }
 
-private let objectConstructor = JSObject.global.Object.function!
-private let arrayConstructor = JSObject.global.Array.function!
+private let _objectConstructor = LazyThreadLocal(initialize: { JSObject.global.Object.function! })
+private var objectConstructor: JSFunction { _objectConstructor.wrappedValue }
+private let _arrayConstructor = LazyThreadLocal(initialize: { JSObject.global.Array.function! })
+private var arrayConstructor: JSFunction { _arrayConstructor.wrappedValue }
 
 #if !hasFeature(Embedded)
 extension Dictionary where Value == ConvertibleToJSValue, Key == String {
@@ -296,4 +298,4 @@ extension Array where Element == ConvertibleToJSValue {
         return _withRawJSValues(self, 0, &_results, body)
     }
 }
-#endif
+#endif

Diff for: Sources/JavaScriptKit/FundamentalObjects/JSSymbol.swift

@@ -1,6 +1,7 @@
 import _CJavaScriptKit
 
-private let Symbol = JSObject.global.Symbol.function!
+private let _Symbol = LazyThreadLocal(initialize: { JSObject.global.Symbol.function! })
+private var Symbol: JSFunction { _Symbol.wrappedValue }
 
 /// A wrapper around [the JavaScript `Symbol`
 /// class](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Symbol)

Diff for: Tests/JavaScriptEventLoopTests/WebWorkerTaskExecutorTests.swift

@@ -38,6 +38,7 @@ final class WebWorkerTaskExecutorTests: XCTestCase {
 
     func testAwaitInsideTask() async throws {
         let executor = try await WebWorkerTaskExecutor(numberOfThreads: 1)
+        defer { executor.terminate() }
 
         let task = Task(executorPreference: executor) {
             await Task.yield()
@@ -46,8 +47,6 @@ final class WebWorkerTaskExecutorTests: XCTestCase {
         }
         let taskRunOnMainThread = try await task.value
         XCTAssertFalse(taskRunOnMainThread)
-
-        executor.terminate()
     }
 
     func testSleepInsideTask() async throws {
@@ -170,6 +169,7 @@ final class WebWorkerTaskExecutorTests: XCTestCase {
         let result = await task.value
         XCTAssertEqual(result, 100)
         XCTAssertEqual(Check.value, 42)
+        executor.terminate()
     }
 
     func testLazyThreadLocalPerThreadInitialization() async throws {
@@ -198,6 +198,72 @@ final class WebWorkerTaskExecutorTests: XCTestCase {
         let result = await task.value
         XCTAssertEqual(result, 100)
         XCTAssertEqual(Check.countOfInitialization, 2)
+        executor.terminate()
+    }
+
+    func testJSValueDecoderOnWorker() async throws {
+        struct DecodeMe: Codable {
+            struct Prop1: Codable {
+                let nested_prop: Int
+            }
+
+            let prop_1: Prop1
+            let prop_2: Int
+            let prop_3: Bool
+            let prop_7: Float
+            let prop_8: String
+            let prop_9: [String]
+        }
+
+        func decodeJob() throws {
+            let json = """
+            {
+                "prop_1": {
+                    "nested_prop": 42
+                },
+                "prop_2": 100,
+                "prop_3": true,
+                "prop_7": 3.14,
+                "prop_8": "Hello, World!",
+                "prop_9": ["a", "b", "c"]
+            }
+            """
+            let object = JSObject.global.JSON.parse(json)
+            let decoder = JSValueDecoder()
+            let result = try decoder.decode(DecodeMe.self, from: object)
+            XCTAssertEqual(result.prop_1.nested_prop, 42)
+            XCTAssertEqual(result.prop_2, 100)
+            XCTAssertEqual(result.prop_3, true)
+            XCTAssertEqual(result.prop_7, 3.14)
+            XCTAssertEqual(result.prop_8, "Hello, World!")
+            XCTAssertEqual(result.prop_9, ["a", "b", "c"])
+        }
+        // Run the job on the main thread first to initialize the object cache
+        try decodeJob()
+
+        let executor = try await WebWorkerTaskExecutor(numberOfThreads: 1)
+        defer { executor.terminate() }
+        let task = Task(executorPreference: executor) {
+            // Run the job on the worker thread to test the object cache
+            // is not shared with the main thread
+            try decodeJob()
+        }
+        try await task.value
+    }
+
+    func testJSArrayCountOnWorker() async throws {
+        let executor = try await WebWorkerTaskExecutor(numberOfThreads: 1)
+        func check() {
+            let object = JSObject.global.Array.function!.new(1, 2, 3, 4, 5)
+            let array = JSArray(object)!
+            XCTAssertEqual(array.count, 5)
+        }
+        check()
+        let task = Task(executorPreference: executor) {
+            check()
+        }
+        await task.value
+        executor.terminate()
     }
 
 /*