From 90f814ac507a174785329c700a1341911113eaed Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 28 Oct 2025 17:49:14 -0400 Subject: [PATCH 1/7] Avoid `unsafeBitCast()` in the new interop target. This PR uses `Unmanaged` to avoid calling `unsafeBitCast()` in the new `_TestingInterop` target on platforms that can use `Atomic`. This improves type safety (yay!) --- .../FallbackEventHandler.swift | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/Sources/_TestingInterop/FallbackEventHandler.swift b/Sources/_TestingInterop/FallbackEventHandler.swift index 2e33cd04d..8284c9900 100644 --- a/Sources/_TestingInterop/FallbackEventHandler.swift +++ b/Sources/_TestingInterop/FallbackEventHandler.swift @@ -27,7 +27,7 @@ private nonisolated(unsafe) let _fallbackEventHandler = { }() #else /// The installed event handler. -private nonisolated(unsafe) let _fallbackEventHandler = Atomic(nil) +private nonisolated(unsafe) let _fallbackEventHandler = Atomic?>(nil) #endif /// A type describing a fallback event handler that testing API can invoke as an @@ -68,7 +68,7 @@ package func _swift_testing_getFallbackEventHandler() -> FallbackEventHandler? { } #else return _fallbackEventHandler.load(ordering: .sequentiallyConsistent).flatMap { fallbackEventHandler in - unsafeBitCast(fallbackEventHandler, to: FallbackEventHandler?.self) + fallbackEventHandler?.takeUnretainedValue() as? FallbackEventHandler } #endif } @@ -86,8 +86,10 @@ package func _swift_testing_getFallbackEventHandler() -> FallbackEventHandler? { @_cdecl("_swift_testing_installFallbackEventHandler") @usableFromInline package func _swift_testing_installFallbackEventHandler(_ handler: FallbackEventHandler) -> CBool { + var result = false + #if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK - return _fallbackEventHandler.withUnsafeMutablePointers { fallbackEventHandler, lock in + result = _fallbackEventHandler.withUnsafeMutablePointers { fallbackEventHandler, lock in os_unfair_lock_lock(lock) defer { os_unfair_lock_unlock(lock) @@ -99,8 +101,15 @@ package func _swift_testing_installFallbackEventHandler(_ handler: FallbackEvent return true } #else - let handler = unsafeBitCast(handler, to: UnsafeRawPointer.self) - return _fallbackEventHandler.compareExchange(expected: nil, desired: handler, ordering: .sequentiallyConsistent).exchanged + let handler = Unmanaged.passRetained(handler as? AnyObject) + defer { + if !result { + handler.release() + } + } + + result = _fallbackEventHandler.compareExchange(expected: nil, desired: handler, ordering: .sequentiallyConsistent).exchanged #endif + return result } #endif From 42b1deea8075dc24d3e5696581570a5b542355d7 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 28 Oct 2025 17:55:26 -0400 Subject: [PATCH 2/7] Fix typo --- Sources/_TestingInterop/FallbackEventHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/_TestingInterop/FallbackEventHandler.swift b/Sources/_TestingInterop/FallbackEventHandler.swift index 8284c9900..332387bbb 100644 --- a/Sources/_TestingInterop/FallbackEventHandler.swift +++ b/Sources/_TestingInterop/FallbackEventHandler.swift @@ -101,7 +101,7 @@ package func _swift_testing_installFallbackEventHandler(_ handler: FallbackEvent return true } #else - let handler = Unmanaged.passRetained(handler as? AnyObject) + let handler = Unmanaged.passRetained(handler as AnyObject) defer { if !result { handler.release() From 303f3e66a6ba1e1c87e7c2910ce4a3e4734f45a5 Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Tue, 28 Oct 2025 17:57:57 -0400 Subject: [PATCH 3/7] Fix another typo --- Sources/_TestingInterop/FallbackEventHandler.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/_TestingInterop/FallbackEventHandler.swift b/Sources/_TestingInterop/FallbackEventHandler.swift index 332387bbb..c7aa87270 100644 --- a/Sources/_TestingInterop/FallbackEventHandler.swift +++ b/Sources/_TestingInterop/FallbackEventHandler.swift @@ -68,7 +68,7 @@ package func _swift_testing_getFallbackEventHandler() -> FallbackEventHandler? { } #else return _fallbackEventHandler.load(ordering: .sequentiallyConsistent).flatMap { fallbackEventHandler in - fallbackEventHandler?.takeUnretainedValue() as? FallbackEventHandler + fallbackEventHandler.takeUnretainedValue() as? FallbackEventHandler } #endif } From d0752719cd6d056293995d60259d9153c98091be Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 29 Oct 2025 11:22:46 -0400 Subject: [PATCH 4/7] Support Embedded Swift and add a package target for building purposes only --- Package.swift | 7 +++++ .../FallbackEventHandler.swift | 26 +++++++++++++------ 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/Package.swift b/Package.swift index 2788502c3..a3c223045 100644 --- a/Package.swift +++ b/Package.swift @@ -209,6 +209,13 @@ let package = Package( cxxSettings: .packageSettings, swiftSettings: .packageSettings + .enableLibraryEvolution() ), + .target( + name: "_TestingInterop_DO_NOT_USE", // just so we can confirm it compiles + dependencies: ["_TestingInternals",], + path: "Sources/_TestingInterop", + cxxSettings: .packageSettings, + swiftSettings: .packageSettings + ), // Cross-import overlays (not supported by Swift Package Manager) .target( diff --git a/Sources/_TestingInterop/FallbackEventHandler.swift b/Sources/_TestingInterop/FallbackEventHandler.swift index c7aa87270..89416ba22 100644 --- a/Sources/_TestingInterop/FallbackEventHandler.swift +++ b/Sources/_TestingInterop/FallbackEventHandler.swift @@ -9,13 +9,13 @@ // #if !SWT_NO_INTEROP -#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK +#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK && !hasFeature(Embedded) private import _TestingInternals #else private import Synchronization #endif -#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK +#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK && !hasFeature(Embedded) /// The installed event handler. private nonisolated(unsafe) let _fallbackEventHandler = { let result = ManagedBuffer.create( @@ -26,8 +26,17 @@ private nonisolated(unsafe) let _fallbackEventHandler = { return result }() #else +/// `Atomic`-compatible storage for ``FallbackEventHandler``. +private final class _FallbackEventHandlerStorage: Sendable, RawRepresentable { + let rawValue: FallbackEventHandler + + init(rawValue: FallbackEventHandler) { + self.rawValue = rawValue + } +} + /// The installed event handler. -private nonisolated(unsafe) let _fallbackEventHandler = Atomic?>(nil) +private let _fallbackEventHandler = Atomic?>(nil) #endif /// A type describing a fallback event handler that testing API can invoke as an @@ -58,7 +67,7 @@ package typealias FallbackEventHandler = @Sendable @convention(c) ( @_cdecl("_swift_testing_getFallbackEventHandler") @usableFromInline package func _swift_testing_getFallbackEventHandler() -> FallbackEventHandler? { -#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK +#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK && !hasFeature(Embedded) return _fallbackEventHandler.withUnsafeMutablePointers { fallbackEventHandler, lock in os_unfair_lock_lock(lock) defer { @@ -67,8 +76,8 @@ package func _swift_testing_getFallbackEventHandler() -> FallbackEventHandler? { return fallbackEventHandler.pointee } #else - return _fallbackEventHandler.load(ordering: .sequentiallyConsistent).flatMap { fallbackEventHandler in - fallbackEventHandler.takeUnretainedValue() as? FallbackEventHandler + return _fallbackEventHandler.load(ordering: .sequentiallyConsistent).map { fallbackEventHandler in + fallbackEventHandler.takeUnretainedValue().rawValue } #endif } @@ -88,7 +97,7 @@ package func _swift_testing_getFallbackEventHandler() -> FallbackEventHandler? { package func _swift_testing_installFallbackEventHandler(_ handler: FallbackEventHandler) -> CBool { var result = false -#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK +#if SWT_TARGET_OS_APPLE && !SWT_NO_OS_UNFAIR_LOCK && !hasFeature(Embedded) result = _fallbackEventHandler.withUnsafeMutablePointers { fallbackEventHandler, lock in os_unfair_lock_lock(lock) defer { @@ -101,7 +110,7 @@ package func _swift_testing_installFallbackEventHandler(_ handler: FallbackEvent return true } #else - let handler = Unmanaged.passRetained(handler as AnyObject) + let handler = Unmanaged.passRetained(_FallbackEventHandlerStorage(rawValue: handler)) defer { if !result { handler.release() @@ -110,6 +119,7 @@ package func _swift_testing_installFallbackEventHandler(_ handler: FallbackEvent result = _fallbackEventHandler.compareExchange(expected: nil, desired: handler, ordering: .sequentiallyConsistent).exchanged #endif + return result } #endif From 083957c600d8b5cc0364d03a65e8ef6b30953d7f Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 29 Oct 2025 11:35:18 -0400 Subject: [PATCH 5/7] Need a product too --- Package.swift | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/Package.swift b/Package.swift index a3c223045..059b39c08 100644 --- a/Package.swift +++ b/Package.swift @@ -105,6 +105,17 @@ let package = Package( ) ) +#if DEBUG + // Build _TestingInterop for debugging/testing purposes only. It is + // important that clients do not link to this product/target. + result += [ + .library( + name: "_TestingInterop_DO_NOT_USE", + targets: ["_TestingInterop_DO_NOT_USE"] + ) + ] +#endif + return result }(), @@ -210,7 +221,9 @@ let package = Package( swiftSettings: .packageSettings + .enableLibraryEvolution() ), .target( - name: "_TestingInterop_DO_NOT_USE", // just so we can confirm it compiles + // Build _TestingInterop for debugging/testing purposes only. It is + // important that clients do not link to this product/target. + name: "_TestingInterop_DO_NOT_USE", dependencies: ["_TestingInternals",], path: "Sources/_TestingInterop", cxxSettings: .packageSettings, From d637dc59e0e1f3433fe3a6746ed47127fb66ed7d Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 29 Oct 2025 11:42:46 -0400 Subject: [PATCH 6/7] Missing exclude --- Package.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Package.swift b/Package.swift index 059b39c08..a82241527 100644 --- a/Package.swift +++ b/Package.swift @@ -226,6 +226,7 @@ let package = Package( name: "_TestingInterop_DO_NOT_USE", dependencies: ["_TestingInternals",], path: "Sources/_TestingInterop", + exclude: ["CMakeLists.txt"], cxxSettings: .packageSettings, swiftSettings: .packageSettings ), From 34fb4f51874186abec3b6d08c5b159d57e32433d Mon Sep 17 00:00:00 2001 From: Jonathan Grynspan Date: Wed, 29 Oct 2025 11:57:35 -0400 Subject: [PATCH 7/7] Add a comment explaining why the non-atomic load is safe --- Sources/_TestingInterop/FallbackEventHandler.swift | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Sources/_TestingInterop/FallbackEventHandler.swift b/Sources/_TestingInterop/FallbackEventHandler.swift index 89416ba22..bd4286315 100644 --- a/Sources/_TestingInterop/FallbackEventHandler.swift +++ b/Sources/_TestingInterop/FallbackEventHandler.swift @@ -76,6 +76,12 @@ package func _swift_testing_getFallbackEventHandler() -> FallbackEventHandler? { return fallbackEventHandler.pointee } #else + // If we had a setter, this load would present a race condition because + // another thread could store a new value in between the load and the call to + // `takeUnretainedValue()`, resulting in a use-after-free on this thread. We + // would need a full lock in order to avoid that problem. However, because we + // instead have a one-time installation function, we can be sure that the + // loaded value (if non-nil) will never be replaced with another value. return _fallbackEventHandler.load(ordering: .sequentiallyConsistent).map { fallbackEventHandler in fallbackEventHandler.takeUnretainedValue().rawValue }