Don't suspend/resume the thread while holding an exclusive inout reference

We pass WorkQueue.queue 'by reference' through inout parameters and across thread suspension points. We noticed a stale value issue in release builds when passing Array with inout, likely due to the copy-on-write mechanism. To avoid stale values after a thread resumes from suspension, sleep the worker thread only after releasing the exclusive inout reference, and re-obtain it after resuming.

Resolves #182

Author: jakepetroules

.github/workflows/pull_request.yml

@@ -28,14 +28,15 @@ jobs:
           # Test dependencies
           yum install -y procps
         fi
-      linux_build_command: 'swift-format lint -s -r --configuration ./.swift-format . && swift test && swift test --disable-default-traits'
+      linux_build_command: 'swift-format lint -s -r --configuration ./.swift-format . && swift test && swift test -c release && swift test --disable-default-traits'
       windows_swift_versions: '["6.1", "nightly-main"]'
       windows_build_command: |
         Invoke-Program swift test
+        #Invoke-Program swift test -c release
         Invoke-Program swift test --disable-default-traits
       enable_macos_checks: true
       macos_xcode_versions: '["16.3"]'
-      macos_build_command: 'xcrun swift-format lint -s -r --configuration ./.swift-format . && xcrun swift test && xcrun swift test --disable-default-traits'
+      macos_build_command: 'xcrun swift-format lint -s -r --configuration ./.swift-format . && xcrun swift test && xcrun swift test -c release && xcrun swift test --disable-default-traits'
       enable_linux_static_sdk_build: true
       linux_static_sdk_versions: '["6.1", "nightly-6.2"]'
       linux_static_sdk_build_command: |

Sources/Subprocess/Platforms/Subprocess+Windows.swift

@@ -1166,8 +1166,20 @@ extension Configuration {
             DeleteProcThreadAttributeList(attributeList)
         }
 
-        var handles = Array(inheritedHandles)
-        handles.withUnsafeMutableBufferPointer { inheritedHandlesPtr in
+        let handles = Array(inheritedHandles)
+
+        // Manually allocate an array instead of using withUnsafeMutableBufferPointer, since the
+        // UpdateProcThreadAttribute documentation states that the lpValue pointer must persist
+        // until the attribute list is destroyed using DeleteProcThreadAttributeList.
+        let handlesPointer = UnsafeMutablePointer<HANDLE>.allocate(capacity: handles.count)
+        defer {
+            handlesPointer.deinitialize(count: handles.count)
+            handlesPointer.deallocate()
+        }
+        handlesPointer.initialize(from: handles, count: handles.count)
+        let inheritedHandlesPtr = UnsafeMutableBufferPointer(start: handlesPointer, count: handles.count)
+
+        do {
             _ = UpdateProcThreadAttribute(
                 attributeList,
                 0,

Sources/Subprocess/Thread.swift

@@ -101,7 +101,8 @@ private final class WorkQueue: Sendable {
     }
 
     private func withUnsafeUnderlyingLock<R>(
-        _ body: (UnsafeMutablePointer<MutexType>, inout [BackgroundWorkItem]) throws -> R
+        condition: (inout [BackgroundWorkItem]) -> Bool = { _ in false },
+        body: (UnsafeMutablePointer<MutexType>, inout [BackgroundWorkItem]) throws -> R
     ) rethrows -> R {
         #if canImport(WinSDK)
         EnterCriticalSection(self.mutex)
@@ -114,20 +115,22 @@ private final class WorkQueue: Sendable {
             pthread_mutex_unlock(mutex)
         }
         #endif
+        if condition(&queue) {
+            #if canImport(WinSDK)
+            SleepConditionVariableCS(self.waitCondition, mutex, INFINITE)
+            #else
+            pthread_cond_wait(self.waitCondition, mutex)
+            #endif
+        }
         return try body(mutex, &queue)
     }
 
     // Only called in worker thread. Sleeps the thread if there's no more item
     func dequeue() -> BackgroundWorkItem? {
-        return self.withUnsafeUnderlyingLock { mutex, queue in
-            if queue.isEmpty {
-                // Sleep the worker thread if there's no more work
-                #if canImport(WinSDK)
-                SleepConditionVariableCS(self.waitCondition, mutex, INFINITE)
-                #else
-                pthread_cond_wait(self.waitCondition, mutex)
-                #endif
-            }
+        return self.withUnsafeUnderlyingLock { queue in
+            // Sleep the worker thread if there's no more work
+            queue.isEmpty
+        } body: { mutex, queue in
             guard !queue.isEmpty else {
                 return nil
             }