Skip to content

Commit 6cf0110

Browse files
authored
Don't use a class to store the current exit test. (#1065)
This PR replaces the private `_CurrentContainer` class used to store the current exit test with a bare pointer. The class is prone to duplicate definitions, but we really just use it as a glorified box type for the move-only `ExitTest`, so an immortal pointer will work just as well. Works around rdar://148837303. ### Checklist: - [x] Code and documentation should follow the style of the [Style Guide](https://github.com/apple/swift-testing/blob/main/Documentation/StyleGuide.md). - [x] If public symbols are renamed or modified, DocC references should be updated.
1 parent 7907a4a commit 6cf0110

File tree

2 files changed

+30
-24
lines changed

2 files changed

+30
-24
lines changed

Sources/Testing/ExitTests/ExitTest.swift

+28-22
Original file line numberDiff line numberDiff line change
@@ -107,31 +107,34 @@ public struct ExitTest: Sendable, ~Copyable {
107107
_observedValues = newValue
108108
}
109109
}
110+
111+
/// Make a copy of this instance.
112+
///
113+
/// - Returns: A copy of this instance.
114+
///
115+
/// This function is unsafe because if the caller is not careful, it could
116+
/// invoke the same exit test twice.
117+
fileprivate borrowing func unsafeCopy() -> Self {
118+
var result = Self(id: id, body: body)
119+
result._observedValues = _observedValues
120+
return result
121+
}
110122
}
111123

112124
#if !SWT_NO_EXIT_TESTS
113125
// MARK: - Current
114126

115127
@_spi(Experimental)
116128
extension ExitTest {
117-
/// A container type to hold the current exit test.
118-
///
119-
/// This class is temporarily necessary until `ManagedBuffer` is updated to
120-
/// support storing move-only values. For more information, see [SE-NNNN](https://github.com/swiftlang/swift-evolution/pull/2657).
121-
private final class _CurrentContainer: Sendable {
122-
/// The exit test represented by this container.
123-
///
124-
/// The value of this property must be optional to avoid a copy when reading
125-
/// the value in ``ExitTest/current``.
126-
let exitTest: ExitTest?
127-
128-
init(exitTest: borrowing ExitTest) {
129-
self.exitTest = ExitTest(id: exitTest.id, body: exitTest.body, _observedValues: exitTest._observedValues)
130-
}
131-
}
132-
133129
/// Storage for ``current``.
134-
private static let _current = Locked<_CurrentContainer?>()
130+
///
131+
/// A pointer is used for indirection because `ManagedBuffer` cannot yet hold
132+
/// move-only types.
133+
private static nonisolated(unsafe) var _current: Locked<UnsafeMutablePointer<ExitTest?>> = {
134+
let current = UnsafeMutablePointer<ExitTest?>.allocate(capacity: 1)
135+
current.initialize(to: nil)
136+
return Locked(rawValue: current)
137+
}()
135138

136139
/// The exit test that is running in the current process, if any.
137140
///
@@ -144,11 +147,13 @@ extension ExitTest {
144147
/// process.
145148
public static var current: ExitTest? {
146149
_read {
147-
if let current = _current.rawValue {
148-
yield current.exitTest
149-
} else {
150-
yield nil
150+
// NOTE: Even though this accessor is `_read` and has borrowing semantics,
151+
// we must make a copy so that we don't yield lock-guarded memory to the
152+
// caller (which is not concurrency-safe.)
153+
let currentCopy = _current.withLock { current in
154+
return current.pointee?.unsafeCopy()
151155
}
156+
yield currentCopy
152157
}
153158
}
154159
}
@@ -235,7 +240,8 @@ extension ExitTest {
235240

236241
// Set ExitTest.current before the test body runs.
237242
Self._current.withLock { current in
238-
current = _CurrentContainer(exitTest: self)
243+
precondition(current.pointee == nil, "Set the current exit test twice in the same process. Please file a bug report at https://github.com/swiftlang/swift-testing/issues/new")
244+
current.pointee = self.unsafeCopy()
239245
}
240246

241247
do {

Sources/Testing/Support/Locked.swift

+2-2
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ struct LockedWith<L, T>: RawRepresentable where L: Lockable {
8888
/// This function can be used to synchronize access to shared data from a
8989
/// synchronous caller. Wherever possible, use actor isolation or other Swift
9090
/// concurrency tools.
91-
nonmutating func withLock<R>(_ body: (inout T) throws -> R) rethrows -> R {
91+
nonmutating func withLock<R>(_ body: (inout T) throws -> R) rethrows -> R where R: ~Copyable {
9292
try _storage.withUnsafeMutablePointers { rawValue, lock in
9393
L.unsafelyAcquireLock(at: lock)
9494
defer {
@@ -118,7 +118,7 @@ struct LockedWith<L, T>: RawRepresentable where L: Lockable {
118118
/// - Warning: Callers that unlock the lock _must_ lock it again before the
119119
/// closure returns. If the lock is not acquired when `body` returns, the
120120
/// effect is undefined.
121-
nonmutating func withUnsafeUnderlyingLock<R>(_ body: (UnsafeMutablePointer<L>, T) throws -> R) rethrows -> R {
121+
nonmutating func withUnsafeUnderlyingLock<R>(_ body: (UnsafeMutablePointer<L>, T) throws -> R) rethrows -> R where R: ~Copyable {
122122
try withLock { value in
123123
try _storage.withUnsafeMutablePointerToElements { lock in
124124
try body(lock, value)

0 commit comments

Comments
 (0)