Skip to content

Commit 476ec77

Browse files
AndrewHoosAndrewHoos
authored
Remove all remaining usages of temp_await and safe_async (#7859)
### Motivation: `temp_await` can cause deadlocks when called from swift concurrency and `safe_async` can cause thread explosion. Replacing the usage of these methods ### Modifications: Vendor topological sort so that we can have an async/await version Replace use of safe_async with withCheckedThrowingContinuation Remove temp_await and safe_async methods Remove uses of unsafe_await in package graph resolution ### Result: No `temp_await` No `safe_async` Only one usage of `unsafe_await` which is exclusively called from the C++ llbuild
1 parent 0f7ab60 commit 476ec77

27 files changed

+269
-287
lines changed

‎Sources/Basics/Concurrency/ConcurrencyHelpers.swift

Lines changed: 0 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -24,31 +24,6 @@ public enum Concurrency {
2424
}
2525
}
2626

27-
// FIXME: mark as deprecated once async/await is available
28-
@available(*, noasync, message: "replace with async/await when available")
29-
@inlinable
30-
public func temp_await<T, ErrorType>(_ body: (@escaping (Result<T, ErrorType>) -> Void) -> Void) throws -> T {
31-
try tsc_await(body)
32-
}
33-
34-
@available(*, noasync, message: "This method blocks the current thread indefinitely. Calling it from the concurrency pool can cause deadlocks")
35-
public func unsafe_await<T>(_ body: @Sendable @escaping () async throws -> T) throws -> T {
36-
let semaphore = DispatchSemaphore(value: 0)
37-
let box = ThreadSafeBox<Result<T, Error>>()
38-
Task {
39-
let localResult: Result<T, Error>
40-
do {
41-
localResult = try await .success(body())
42-
} catch {
43-
localResult = .failure(error)
44-
}
45-
box.mutate { _ in localResult }
46-
semaphore.signal()
47-
}
48-
semaphore.wait()
49-
return try box.get()!.get()
50-
}
51-
5227
@available(*, noasync, message: "This method blocks the current thread indefinitely. Calling it from the concurrency pool can cause deadlocks")
5328
public func unsafe_await<T>(_ body: @Sendable @escaping () async -> T) -> T {
5429
let semaphore = DispatchSemaphore(value: 0)
@@ -63,12 +38,6 @@ public func unsafe_await<T>(_ body: @Sendable @escaping () async -> T) -> T {
6338
return box.get()!
6439
}
6540

66-
// FIXME: mark as deprecated once async/await is available
67-
@available(*, deprecated, message: "replace with async/await when available")
68-
@inlinable
69-
public func temp_await<T>(_ body: (@escaping (T) -> Void) -> Void) -> T {
70-
tsc_await(body)
71-
}
7241

7342
extension DispatchQueue {
7443
// a shared concurrent queue for running concurrent asynchronous operations
@@ -78,35 +47,6 @@ extension DispatchQueue {
7847
)
7948
}
8049

81-
/// Bridges between potentially blocking methods that take a result completion closure and async/await
82-
public func safe_async<T, ErrorType: Error>(
83-
_ body: @escaping @Sendable (@escaping @Sendable (Result<T, ErrorType>) -> Void) -> Void
84-
) async throws -> T {
85-
try await withCheckedThrowingContinuation { continuation in
86-
// It is possible that body make block indefinitely on a lock, semaphore,
87-
// or similar then synchronously call the completion handler. For full safety
88-
// it is essential to move the execution off the swift concurrency pool
89-
DispatchQueue.sharedConcurrent.async {
90-
body {
91-
continuation.resume(with: $0)
92-
}
93-
}
94-
}
95-
}
96-
97-
/// Bridges between potentially blocking methods that take a result completion closure and async/await
98-
public func safe_async<T>(_ body: @escaping @Sendable (@escaping (Result<T, Never>) -> Void) -> Void) async -> T {
99-
await withCheckedContinuation { continuation in
100-
// It is possible that body make block indefinitely on a lock, semaphore,
101-
// or similar then synchronously call the completion handler. For full safety
102-
// it is essential to move the execution off the swift concurrency pool
103-
DispatchQueue.sharedConcurrent.async {
104-
body {
105-
continuation.resume(with: $0)
106-
}
107-
}
108-
}
109-
}
11050

11151
#if !canImport(Darwin)
11252
// As of Swift 5.7 and 5.8 swift-corelibs-foundation doesn't have `Sendable` annotations yet.

‎Sources/Basics/HTTPClient/LegacyHTTPClient.swift

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13+
import _Concurrency
1314
import Dispatch
1415
import struct Foundation.Data
1516
import struct Foundation.Date
@@ -313,8 +314,8 @@ extension LegacyHTTPClient {
313314
options: Request.Options = .init(),
314315
observabilityScope: ObservabilityScope? = .none
315316
) async throws -> Response {
316-
try await safe_async {
317-
self.head(url, headers: headers, options: options, completion: $0)
317+
try await withCheckedThrowingContinuation {
318+
self.head(url, headers: headers, options: options, completion: $0.resume(with:))
318319
}
319320
}
320321
@available(*, noasync, message: "Use the async alternative")
@@ -338,8 +339,8 @@ extension LegacyHTTPClient {
338339
options: Request.Options = .init(),
339340
observabilityScope: ObservabilityScope? = .none
340341
) async throws -> Response {
341-
try await safe_async {
342-
self.get(url, headers: headers, options: options, completion: $0)
342+
try await withCheckedThrowingContinuation {
343+
self.get(url, headers: headers, options: options, completion: $0.resume(with:))
343344
}
344345
}
345346
@available(*, noasync, message: "Use the async alternative")

‎Sources/Build/BuildOperation.swift

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
145145
private let config: LLBuildSystemConfiguration
146146

147147
/// The closure for loading the package graph.
148-
let packageGraphLoader: () throws -> ModulesGraph
148+
let packageGraphLoader: () async throws -> ModulesGraph
149149

150150
/// the plugin configuration for build plugins
151151
let pluginConfiguration: PluginConfiguration?
@@ -233,7 +233,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
233233
productsBuildParameters: BuildParameters,
234234
toolsBuildParameters: BuildParameters,
235235
cacheBuildManifest: Bool,
236-
packageGraphLoader: @escaping () throws -> ModulesGraph,
236+
packageGraphLoader: @escaping () async throws -> ModulesGraph,
237237
pluginConfiguration: PluginConfiguration? = .none,
238238
scratchDirectory: AbsolutePath,
239239
traitConfiguration: TraitConfiguration?,
@@ -269,10 +269,9 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
269269
self.pkgConfigDirectories = pkgConfigDirectories
270270
}
271271

272-
@available(*, noasync, message: "This must only be called from a dispatch queue")
273-
public func getPackageGraph() throws -> ModulesGraph {
274-
try self.packageGraph.memoize {
275-
try self.packageGraphLoader()
272+
public func getPackageGraph() async throws -> ModulesGraph {
273+
try await self.packageGraph.memoize {
274+
try await self.packageGraphLoader()
276275
}
277276
}
278277

@@ -390,7 +389,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
390389
}
391390

392391
/// Perform a build using the given build description and subset.
393-
public func build(subset: BuildSubset) throws {
392+
public func build(subset: BuildSubset) async throws {
394393
guard !self.config.shouldSkipBuilding(for: .target) else {
395394
return
396395
}
@@ -400,9 +399,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
400399
// Get the build description (either a cached one or newly created).
401400

402401
// Get the build description
403-
let buildDescription = try unsafe_await {
404-
try await self.getBuildDescription(subset: subset)
405-
}
402+
let buildDescription = try await self.getBuildDescription(subset: subset)
406403

407404
// Verify dependency imports on the described targets
408405
try verifyTargetImports(in: buildDescription)
@@ -417,7 +414,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
417414
// If any plugins are part of the build set, compile them now to surface
418415
// any errors up-front. Returns true if we should proceed with the build
419416
// or false if not. It will already have thrown any appropriate error.
420-
guard try unsafe_await({ try await self.compilePlugins(in: subset) }) else {
417+
guard try await self.compilePlugins(in: subset) else {
421418
return
422419
}
423420

@@ -426,7 +423,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
426423
progressTracker.buildStart(configuration: configuration)
427424

428425
// Perform the build.
429-
let llbuildTarget = try computeLLBuildTargetName(for: subset)
426+
let llbuildTarget = try await computeLLBuildTargetName(for: subset)
430427
let success = buildSystem.build(target: llbuildTarget)
431428

432429
let duration = buildStartTime.distance(to: .now())
@@ -577,15 +574,15 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
577574
}
578575

579576
/// Compute the llbuild target name using the given subset.
580-
func computeLLBuildTargetName(for subset: BuildSubset) throws -> String {
577+
func computeLLBuildTargetName(for subset: BuildSubset) async throws -> String {
581578
switch subset {
582579
case .allExcludingTests:
583580
return LLBuildManifestBuilder.TargetKind.main.targetName
584581
case .allIncludingTests:
585582
return LLBuildManifestBuilder.TargetKind.test.targetName
586583
case .product(let productName, let destination):
587584
// FIXME: This is super unfortunate that we might need to load the package graph.
588-
let graph = try getPackageGraph()
585+
let graph = try await getPackageGraph()
589586

590587
let buildTriple: BuildTriple? = if let destination {
591588
destination == .host ? .tools : .destination
@@ -619,7 +616,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
619616
return try product.getLLBuildTargetName(buildParameters: buildParameters)
620617
case .target(let targetName, let destination):
621618
// FIXME: This is super unfortunate that we might need to load the package graph.
622-
let graph = try getPackageGraph()
619+
let graph = try await getPackageGraph()
623620

624621
let buildTriple: BuildTriple? = if let destination {
625622
destination == .host ? .tools : .destination
@@ -648,7 +645,7 @@ public final class BuildOperation: PackageStructureDelegate, SPMBuildCore.BuildS
648645
/// Create the build plan and return the build description.
649646
private func plan(subset: BuildSubset? = nil) async throws -> (description: BuildDescription, manifest: LLBuildManifest) {
650647
// Load the package graph.
651-
let graph = try getPackageGraph()
648+
let graph = try await getPackageGraph()
652649

653650
let pluginTools: [ResolvedModule.ID: [String: PluginTool]]
654651
// FIXME: This is unfortunate but we need to build plugin tools upfront at the moment because
@@ -919,8 +916,8 @@ extension BuildOperation {
919916
config: config
920917
)
921918

922-
func buildToolBuilder(_ name: String, _ path: RelativePath) throws -> AbsolutePath? {
923-
let llbuildTarget = try self.computeLLBuildTargetName(for: .product(name, for: .host))
919+
func buildToolBuilder(_ name: String, _ path: RelativePath) async throws -> AbsolutePath? {
920+
let llbuildTarget = try await self.computeLLBuildTargetName(for: .product(name, for: .host))
924921
let success = buildSystem.build(target: llbuildTarget)
925922

926923
if !success {
@@ -936,12 +933,12 @@ extension BuildOperation {
936933
for plugin in plugins where accessibleToolsPerPlugin[plugin.id] == nil {
937934
// Determine the tools to which this plugin has access, and create a name-to-path mapping from tool
938935
// names to the corresponding paths. Built tools are assumed to be in the build tools directory.
939-
let accessibleTools = try plugin.preparePluginTools(
936+
let accessibleTools = try await plugin.preparePluginTools(
940937
fileSystem: fileSystem,
941938
environment: config.buildEnvironment(for: .host),
942939
for: hostTriple
943940
) { name, path in
944-
if let result = try buildToolBuilder(name, path) {
941+
if let result = try await buildToolBuilder(name, path) {
945942
return result
946943
} else {
947944
return config.buildPath(for: .host).appending(path)

‎Sources/Commands/PackageCommands/APIDiff.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,14 +92,14 @@ struct APIDiff: AsyncSwiftCommand {
9292
cacheBuildManifest: false
9393
)
9494

95-
let packageGraph = try buildSystem.getPackageGraph()
95+
let packageGraph = try await buildSystem.getPackageGraph()
9696
let modulesToDiff = try determineModulesToDiff(
9797
packageGraph: packageGraph,
9898
observabilityScope: swiftCommandState.observabilityScope
9999
)
100100

101101
// Build the current package.
102-
try buildSystem.build()
102+
try await buildSystem.build()
103103

104104
// Dump JSON for the baseline package.
105105
let baselineDumper = try APIDigesterBaselineDumper(

‎Sources/Commands/PackageCommands/DumpCommands.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ struct DumpSymbolGraph: AsyncSwiftCommand {
5353
traitConfiguration: .init(enableAllTraits: true),
5454
cacheBuildManifest: false
5555
)
56-
try buildSystem.build()
56+
try await buildSystem.build()
5757

5858
// Configure the symbol graph extractor.
5959
let symbolGraphExtractor = try SymbolGraphExtract(
@@ -71,7 +71,7 @@ struct DumpSymbolGraph: AsyncSwiftCommand {
7171
// Run the tool once for every library and executable target in the root package.
7272
let buildPlan = try buildSystem.buildPlan
7373
let symbolGraphDirectory = buildPlan.destinationBuildParameters.dataPath.appending("symbolgraph")
74-
let targets = try buildSystem.getPackageGraph().rootPackages.flatMap{ $0.modules }.filter{ $0.type == .library }
74+
let targets = try await buildSystem.getPackageGraph().rootPackages.flatMap{ $0.modules }.filter{ $0.type == .library }
7575
for target in targets {
7676
print("-- Emitting symbol graph for", target.name)
7777
let result = try symbolGraphExtractor.extractSymbolGraph(

‎Sources/Commands/PackageCommands/PluginCommand.swift

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import ArgumentParser
1414
import Basics
15+
import _Concurrency
1516
import CoreCommands
1617
import Dispatch
1718

@@ -333,13 +334,13 @@ struct PluginCommand: AsyncSwiftCommand {
333334
packageGraphLoader: { packageGraph }
334335
)
335336

336-
let accessibleTools = try plugin.preparePluginTools(
337+
let accessibleTools = try await plugin.preparePluginTools(
337338
fileSystem: swiftCommandState.fileSystem,
338339
environment: buildParameters.buildEnvironment,
339340
for: try pluginScriptRunner.hostTriple
340341
) { name, _ in
341342
// Build the product referenced by the tool, and add the executable to the tool map. Product dependencies are not supported within a package, so if the tool happens to be from the same package, we instead find the executable that corresponds to the product. There is always one, because of autogeneration of implicit executables with the same name as the target if there isn't an explicit one.
342-
try buildSystem.build(subset: .product(name, for: .host))
343+
try await buildSystem.build(subset: .product(name, for: .host))
343344
if let builtTool = try buildSystem.buildPlan.buildProducts.first(where: {
344345
$0.product.name == name && $0.buildParameters.destination == .host
345346
}) {
@@ -360,7 +361,7 @@ struct PluginCommand: AsyncSwiftCommand {
360361
let allowNetworkConnectionsCopy = allowNetworkConnections
361362

362363
let buildEnvironment = buildParameters.buildEnvironment
363-
let _ = try await safe_async { pluginTarget.invoke(
364+
let _ = try await withCheckedThrowingContinuation { pluginTarget.invoke(
364365
action: .performCommand(package: package, arguments: arguments),
365366
buildEnvironment: buildEnvironment,
366367
scriptRunner: pluginScriptRunner,
@@ -378,7 +379,7 @@ struct PluginCommand: AsyncSwiftCommand {
378379
observabilityScope: swiftCommandState.observabilityScope,
379380
callbackQueue: delegateQueue,
380381
delegate: pluginDelegate,
381-
completion: $0
382+
completion: $0.resume(with:)
382383
) }
383384

384385
// TODO: We should also emit a final line of output regarding the result.

‎Sources/Commands/Snippets/Cards/SnippetCard.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,9 +94,9 @@ struct SnippetCard: Card {
9494
func runExample() async throws {
9595
print("Building '\(snippet.path)'\n")
9696
let buildSystem = try await swiftCommandState.createBuildSystem(explicitProduct: snippet.name, traitConfiguration: .init())
97-
try buildSystem.build(subset: .product(snippet.name))
97+
try await buildSystem.build(subset: .product(snippet.name))
9898
let executablePath = try swiftCommandState.productsBuildParameters.buildPath.appending(component: snippet.name)
99-
if let exampleTarget = try buildSystem.getPackageGraph().module(for: snippet.name, destination: .destination) {
99+
if let exampleTarget = try await buildSystem.getPackageGraph().module(for: snippet.name, destination: .destination) {
100100
try ProcessEnv.chdir(exampleTarget.sources.paths[0].parentDirectory)
101101
}
102102
try exec(path: executablePath.pathString, args: [])

‎Sources/Commands/SwiftBuildCommand.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ public struct SwiftBuildCommand: AsyncSwiftCommand {
183183
outputStream: TSCBasic.stdoutStream
184184
)
185185
do {
186-
try buildSystem.build(subset: subset)
186+
try await buildSystem.build(subset: subset)
187187
} catch _ as Diagnostics {
188188
throw ExitCode.failure
189189
}

‎Sources/Commands/SwiftRunCommand.swift

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -125,11 +125,9 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
125125
case .repl:
126126
// Load a custom package graph which has a special product for REPL.
127127
let asyncUnsafeGraphLoader = {
128-
try unsafe_await {
129-
try await swiftCommandState.loadPackageGraph(
130-
explicitProduct: self.options.executable
131-
)
132-
}
128+
try await swiftCommandState.loadPackageGraph(
129+
explicitProduct: self.options.executable
130+
)
133131
}
134132

135133
// Construct the build operation.
@@ -142,7 +140,7 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
142140
)
143141

144142
// Perform build.
145-
try buildSystem.build()
143+
try await buildSystem.build()
146144

147145
// Execute the REPL.
148146
let arguments = try buildSystem.buildPlan.createREPLArguments()
@@ -160,11 +158,11 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
160158
explicitProduct: options.executable,
161159
traitConfiguration: .init(traitOptions: self.options.traits)
162160
)
163-
let productName = try findProductName(in: buildSystem.getPackageGraph())
161+
let productName = try await findProductName(in: buildSystem.getPackageGraph())
164162
if options.shouldBuildTests {
165-
try buildSystem.build(subset: .allIncludingTests)
163+
try await buildSystem.build(subset: .allIncludingTests)
166164
} else if options.shouldBuild {
167-
try buildSystem.build(subset: .product(productName))
165+
try await buildSystem.build(subset: .product(productName))
168166
}
169167

170168
let executablePath = try swiftCommandState.productsBuildParameters.buildPath.appending(component: productName)
@@ -205,11 +203,11 @@ public struct SwiftRunCommand: AsyncSwiftCommand {
205203
explicitProduct: options.executable,
206204
traitConfiguration: .init(traitOptions: self.options.traits)
207205
)
208-
let productName = try findProductName(in: buildSystem.getPackageGraph())
206+
let productName = try await findProductName(in: buildSystem.getPackageGraph())
209207
if options.shouldBuildTests {
210-
try buildSystem.build(subset: .allIncludingTests)
208+
try await buildSystem.build(subset: .allIncludingTests)
211209
} else if options.shouldBuild {
212-
try buildSystem.build(subset: .product(productName))
210+
try await buildSystem.build(subset: .product(productName))
213211
}
214212

215213
let executablePath = try swiftCommandState.productsBuildParameters.buildPath.appending(component: productName)

0 commit comments

Comments
 (0)