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/Error.swift

@@ -51,6 +51,7 @@ extension SubprocessError {
             case failedToResume
             case failedToCreatePipe
             case invalidWindowsPath(String)
+            case failedToCreateProcessThreadAttributeList
         }
 
         /// The numeric value of this code.
@@ -86,6 +87,8 @@ extension SubprocessError {
                 return 13
             case .invalidWindowsPath(_):
                 return 14
+            case .failedToCreateProcessThreadAttributeList:
+                return 15
             }
         }
 
@@ -132,6 +135,8 @@ extension SubprocessError: CustomStringConvertible, CustomDebugStringConvertible
             return "Failed to create a pipe to communicate to child process."
         case .invalidWindowsPath(let badPath):
             return "\"\(badPath)\" is not a valid Windows path."
+        case .failedToCreateProcessThreadAttributeList:
+            return "Failed to create process thread attribute list."
         }
     }
 

Sources/Subprocess/Platforms/Subprocess+Windows.swift

@@ -155,7 +155,10 @@ extension Configuration {
                                         spawnContext.startupInfo.pointer(to: \.StartupInfo)!,
                                         &processInfo
                                     )
-                                    return (result, processInfo, GetLastError())
+                                    if result == ERROR_INVALID_PARAMETER {
+                                        fatalError("applicationNameW=\(applicationNameW), commandAndArgsW=\(commandAndArgsW), spawnContext.createProcessFlags=\(spawnContext.createProcessFlags), environmentW=\(environmentW), intendedWorkingDirW=\(intendedWorkingDirW), StartupInfo=\(spawnContext.startupInfo.pointer(to: \.StartupInfo)!)")
+                                    }
+                                    return (result, processInfo, result)
                                 }
                             }
                         }

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
             }