Skip to content

Commit ed79161

Browse files
stmontgomerystmontgomery
authored
Avoid runtime collisions when multiple copies of the testing library are loaded (#1471)
This PR avoids runtime collisions when multiple copies of the testing library are loaded into a given process by introducing the ability to customize the module ABI name of each copy. ### Motivation: In some usage scenarios there can be two or more copies of the testing library loaded into a runner process. When this happens on platforms such as Darwin which use the Objective-C runtime, this can cause duplicate class definition warnings to be logged to the console because (non-generic) class and actor types are implemented in the Objective-C runtime as classes. For a recent example where this occurred and was worked around, see #1444. Although we have not seen practical reports of this, a similar problem could arise at build/link time on some platforms due to exported symbols from the testing library having multiple definitions. Resolves rdar://148912491 ### Modifications: This PR specifies a unique module ABI name for each public library module by passing `-module-abi-name <name>` to the Swift compiler. It does this differently depending on the build context: - When building as a Swift package, this PR appends a hardcoded suffix (`_package`) to the name of each module to form its ABI name. It does this by default for package builds because package copies of the testing library are virtually always loaded by runner processes which also load a prebuilt copy of the testing library, meaning that a package copy is most often considered a "secondary" copy which needs to avoid runtime conflicts with a primary copy. - When building via CMake, this PR adds the _ability_ to define a custom ABI name suffix, and this allows the client performing the build to control the ABI name. In particular, this allows a build intended for inclusion in an official Swift toolchain to specify a suffix, but allows custom CMake builds to remain unsuffixed. ### 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 4e6f213 commit ed79161

15 files changed

Lines changed: 82 additions & 26 deletions

File tree

‎Package.swift‎

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ let package = Package(
141141
],
142142
exclude: ["CMakeLists.txt", "Testing.swiftcrossimport"],
143143
cxxSettings: .packageSettings,
144-
swiftSettings: .packageSettings + .enableLibraryEvolution(),
144+
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("Testing"),
145145
linkerSettings: [
146146
.linkedLibrary("execinfo", .when(platforms: [.custom("freebsd"), .openbsd]))
147147
]
@@ -189,21 +189,15 @@ let package = Package(
189189
.product(name: "SwiftCompilerPlugin", package: "swift-syntax"),
190190
],
191191
exclude: ["CMakeLists.txt"],
192-
swiftSettings: .packageSettings + {
193-
var result = [PackageDescription.SwiftSetting]()
194-
192+
swiftSettings: .packageSettings + [
195193
// The only target which needs the ability to import this macro
196194
// implementation target's module is its unit test target. Users of the
197195
// macros this target implements use them via their declarations in the
198196
// Testing module. This target's module is never distributed to users,
199197
// but as an additional guard against accidental misuse, this specifies
200198
// the unit test target as the only allowable client.
201-
if buildingForDevelopment {
202-
result.append(.unsafeFlags(["-Xfrontend", "-allowable-client", "-Xfrontend", "TestingMacrosTests"]))
203-
}
204-
205-
return result
206-
}()
199+
.unsafeFlags(["-Xfrontend", "-allowable-client", "-Xfrontend", "TestingMacrosTests"]),
200+
]
207201
),
208202

209203
// "Support" targets: These targets are not meant to be used directly by
@@ -218,7 +212,7 @@ let package = Package(
218212
dependencies: ["_TestingInternals",],
219213
exclude: ["CMakeLists.txt"],
220214
cxxSettings: .packageSettings,
221-
swiftSettings: .packageSettings + .enableLibraryEvolution()
215+
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_TestDiscovery")
222216
),
223217
.target(
224218
// Build _TestingInterop for debugging/testing purposes only. It is
@@ -228,7 +222,7 @@ let package = Package(
228222
path: "Sources/_TestingInterop",
229223
exclude: ["CMakeLists.txt"],
230224
cxxSettings: .packageSettings,
231-
swiftSettings: .packageSettings
225+
swiftSettings: .packageSettings + .moduleABIName("_TestingInterop")
232226
),
233227

234228
// Cross-import overlays (not supported by Swift Package Manager)
@@ -240,7 +234,7 @@ let package = Package(
240234
],
241235
path: "Sources/Overlays/_Testing_AppKit",
242236
exclude: ["CMakeLists.txt"],
243-
swiftSettings: .packageSettings + .enableLibraryEvolution()
237+
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("Testing")
244238
),
245239
.target(
246240
name: "_Testing_CoreGraphics",
@@ -249,7 +243,7 @@ let package = Package(
249243
],
250244
path: "Sources/Overlays/_Testing_CoreGraphics",
251245
exclude: ["CMakeLists.txt"],
252-
swiftSettings: .packageSettings + .enableLibraryEvolution()
246+
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_CoreGraphics")
253247
),
254248
.target(
255249
name: "_Testing_CoreImage",
@@ -259,7 +253,7 @@ let package = Package(
259253
],
260254
path: "Sources/Overlays/_Testing_CoreImage",
261255
exclude: ["CMakeLists.txt"],
262-
swiftSettings: .packageSettings + .enableLibraryEvolution()
256+
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_CoreImage")
263257
),
264258
.target(
265259
name: "_Testing_Foundation",
@@ -271,7 +265,7 @@ let package = Package(
271265
// The Foundation module only has Library Evolution enabled on Apple
272266
// platforms, and since this target's module publicly imports Foundation,
273267
// it can only enable Library Evolution itself on those platforms.
274-
swiftSettings: .packageSettings + .enableLibraryEvolution(.whenApple())
268+
swiftSettings: .packageSettings + .enableLibraryEvolution(.whenApple()) + .moduleABIName("_Testing_Foundation")
275269
),
276270
.target(
277271
name: "_Testing_UIKit",
@@ -282,7 +276,7 @@ let package = Package(
282276
],
283277
path: "Sources/Overlays/_Testing_UIKit",
284278
exclude: ["CMakeLists.txt"],
285-
swiftSettings: .packageSettings + .enableLibraryEvolution()
279+
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_UIKit")
286280
),
287281
.target(
288282
name: "_Testing_WinSDK",
@@ -291,7 +285,7 @@ let package = Package(
291285
],
292286
path: "Sources/Overlays/_Testing_WinSDK",
293287
exclude: ["CMakeLists.txt"],
294-
swiftSettings: .packageSettings + .enableLibraryEvolution()
288+
swiftSettings: .packageSettings + .enableLibraryEvolution() + .moduleABIName("_Testing_WinSDK")
295289
),
296290

297291
// Utility targets: These are utilities intended for use when developing
@@ -448,6 +442,30 @@ extension Array where Element == PackageDescription.SwiftSetting {
448442

449443
return result
450444
}
445+
446+
/// Create a Swift setting which specifies the module ABI name to use when
447+
/// building the target with the specified name.
448+
///
449+
/// - Parameters:
450+
/// - targetName The name of the target for which an ABI name should be
451+
/// specified.
452+
///
453+
/// - Returns: A Swift setting that specifies the ABI name of the module of
454+
/// the target named `targetName`.
455+
///
456+
/// This function simplifies the process of specifying a custom module ABI
457+
/// name for various targets in this package. The module ABI name is given a
458+
/// suffix for all targets in this package which emit a module that is also
459+
/// included in the built-in copy of Swift Testing in Swift toolchains and
460+
/// vendor distributions. Without this, there can be runtime collisions; for
461+
/// example, on Darwin platforms (where Swift uses the Objective-C runtime),
462+
/// a non-generic Swift class type causes a warning from the runtime about
463+
/// duplicate class definitions. Specifying a distinct ABI name for each
464+
/// module related to Swift Testing loaded into a runner process avoids this
465+
/// issue.
466+
static func moduleABIName(_ targetName: String) -> Self {
467+
[.unsafeFlags(["-module-abi-name", "\(targetName)_package"])]
468+
}
451469
}
452470

453471
extension Array where Element == PackageDescription.CXXSetting {

‎Sources/Overlays/_Testing_AppKit/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# See https://swift.org/CONTRIBUTORS.txt for Swift project authors
88

99
if (CMAKE_SYSTEM_NAME STREQUAL "Darwin")
10+
include(ModuleABIName)
1011
add_library(_Testing_AppKit
1112
Attachments/NSImage+AttachableAsImage.swift
1213
ReexportTesting.swift)

‎Sources/Overlays/_Testing_CoreGraphics/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# See https://swift.org/CONTRIBUTORS.txt for Swift project authors
88

99
if(APPLE)
10+
include(ModuleABIName)
1011
add_library(_Testing_CoreGraphics
1112
Attachments/AttachableAsCGImage.swift
1213
Attachments/_AttachableImageWrapper+AttachableWrapper.swift

‎Sources/Overlays/_Testing_CoreImage/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# See https://swift.org/CONTRIBUTORS.txt for Swift project authors
88

99
if(APPLE)
10+
include(ModuleABIName)
1011
add_library(_Testing_CoreImage
1112
Attachments/CIImage+AttachableAsImage.swift
1213
ReexportTesting.swift)

‎Sources/Overlays/_Testing_Foundation/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
# See https://swift.org/LICENSE.txt for license information
77
# See https://swift.org/CONTRIBUTORS.txt for Swift project authors
88

9+
include(ModuleABIName)
910
add_library(_Testing_Foundation
1011
Attachments/_AttachableURLWrapper.swift
1112
Attachments/EncodingFormat.swift

‎Sources/Overlays/_Testing_UIKit/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# See https://swift.org/CONTRIBUTORS.txt for Swift project authors
88

99
if(APPLE)
10+
include(ModuleABIName)
1011
add_library(_Testing_UIKit
1112
Attachments/UIImage+AttachableAsImage.swift
1213
ReexportTesting.swift)

‎Sources/Overlays/_Testing_WinSDK/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
# See https://swift.org/CONTRIBUTORS.txt for Swift project authors
88

99
if (CMAKE_SYSTEM_NAME STREQUAL "Windows")
10+
include(ModuleABIName)
1011
add_library(_Testing_WinSDK
1112
Attachments/_AttachableImageWrapper+AttachableWrapper.swift
1213
Attachments/AttachableAsIWICBitmapSource.swift

‎Sources/Testing/CMakeLists.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
# See https://swift.org/LICENSE.txt for license information
77
# See https://swift.org/CONTRIBUTORS.txt for Swift project authors
88

9+
include(ModuleABIName)
910
add_library(Testing
1011
ABI/EntryPoints/ABIEntryPoint.swift
1112
ABI/EntryPoints/EntryPoint.swift

‎Sources/Testing/Running/Runner.swift‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ extension Runner {
7777
/// type at runtime, it may be better-suited for ``Configuration`` instead.
7878
private struct _Context: Sendable {
7979
/// A serializer used to reduce parallelism among test cases.
80-
var testCaseSerializer: Serializer<Void>?
80+
var testCaseSerializer: Serializer?
8181
}
8282

8383
/// Apply the custom scope for any test scope providers of the traits

‎Sources/Testing/Support/Serializer.swift‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,13 +42,8 @@ var defaultParallelizationWidth: Int {
4242
/// items do not start running; they must wait until the suspended work item
4343
/// either returns or throws an error.
4444
///
45-
/// The generic type parameter `T` is unused. It avoids warnings when multiple
46-
/// copies of the testing library are loaded into a runner process on platforms
47-
/// which use the Objective-C runtime, due to non-generic actor types being
48-
/// implemented as classes there.
49-
///
5045
/// This type is not part of the public interface of the testing library.
51-
final actor Serializer<T> {
46+
final actor Serializer {
5247
/// The maximum number of work items that may run concurrently.
5348
nonisolated let maximumWidth: Int
5449

0 commit comments

Comments
 (0)