Skip to content

Commit 085b1ab

Browse files
committed
Update WorkQueue.queue to be a reference type instead of an array.
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, switch to a dedicated reference type Queue.
1 parent a040f9b commit 085b1ab

File tree

2 files changed

+27
-6
lines changed

2 files changed

+27
-6
lines changed

.github/workflows/pull_request.yml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,14 +28,14 @@ jobs:
2828
# Test dependencies
2929
yum install -y procps
3030
fi
31-
linux_build_command: 'swift-format lint -s -r --configuration ./.swift-format . && swift test && swift test --disable-default-traits'
31+
linux_build_command: 'swift-format lint -s -r --configuration ./.swift-format . && swift test && swift test -c release && swift test --disable-default-traits'
3232
windows_swift_versions: '["6.1", "nightly-main"]'
3333
windows_build_command: |
3434
Invoke-Program swift test
3535
Invoke-Program swift test --disable-default-traits
3636
enable_macos_checks: true
3737
macos_xcode_versions: '["16.3"]'
38-
macos_build_command: 'xcrun swift-format lint -s -r --configuration ./.swift-format . && xcrun swift test && xcrun swift test --disable-default-traits'
38+
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'
3939
enable_linux_static_sdk_build: true
4040
linux_static_sdk_versions: '["6.1", "nightly-6.2"]'
4141
linux_static_sdk_build_command: |

Sources/Subprocess/Thread.swift

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,33 @@ private struct BackgroundWorkItem {
7777
// We can't use Mutex directly here because we need the underlying `pthread_mutex_t` to be
7878
// exposed so we can use it with `pthread_cond_wait`.
7979
private final class WorkQueue: Sendable {
80-
private nonisolated(unsafe) var queue: [BackgroundWorkItem]
80+
// Queue needs to be a reference type because we pass it inout
81+
final class Queue: Sendable {
82+
internal nonisolated(unsafe) var queue: [BackgroundWorkItem]
83+
84+
var isEmpty: Bool { self.queue.isEmpty }
85+
86+
func append(_ item: BackgroundWorkItem) {
87+
self.queue.append(item)
88+
}
89+
90+
func removeFirst() -> BackgroundWorkItem {
91+
return self.queue.removeFirst()
92+
}
93+
94+
func removeAll() { self.queue.removeAll() }
95+
96+
init() {
97+
self.queue = []
98+
}
99+
}
100+
101+
private nonisolated(unsafe) var queue: Queue
81102
internal nonisolated(unsafe) let mutex: UnsafeMutablePointer<MutexType>
82103
internal nonisolated(unsafe) let waitCondition: UnsafeMutablePointer<ConditionType>
83104

84105
init() {
85-
self.queue = []
106+
self.queue = Queue()
86107
self.mutex = UnsafeMutablePointer<MutexType>.allocate(capacity: 1)
87108
self.waitCondition = UnsafeMutablePointer<ConditionType>.allocate(capacity: 1)
88109
#if canImport(WinSDK)
@@ -94,14 +115,14 @@ private final class WorkQueue: Sendable {
94115
#endif
95116
}
96117

97-
func withLock<R>(_ body: (inout [BackgroundWorkItem]) throws -> R) rethrows -> R {
118+
func withLock<R>(_ body: (inout Queue) throws -> R) rethrows -> R {
98119
try withUnsafeUnderlyingLock { _, queue in
99120
try body(&queue)
100121
}
101122
}
102123

103124
private func withUnsafeUnderlyingLock<R>(
104-
_ body: (UnsafeMutablePointer<MutexType>, inout [BackgroundWorkItem]) throws -> R
125+
_ body: (UnsafeMutablePointer<MutexType>, inout Queue) throws -> R
105126
) rethrows -> R {
106127
#if canImport(WinSDK)
107128
EnterCriticalSection(self.mutex)

0 commit comments

Comments
 (0)