Skip to content

Commit 1764337

Browse files
AndrewHoosAndrewHoos
authored
Remove temp_await and safe_async throughout the entire codebase [part 1] (#7761)
Remove both `temp_await` and `safe_async` in favor of broader adoption of `async`/`await`. ### Motivation: `temp_await` is very dangerous API that blocks the current thread and is not marked as unavailable from async methods. The only need for temp_await is because LLBuild only supports non-async methods so any llbuild commands that call async methods have to block waiting. Because `temp_await` is liberally spread across the code base there are a number of async methods that may indirectly call into `temp_await` and indefinitely block a swift concurrency thread. This risks a deadlock. To prevent the deadlock the `safe_async` method dispatches to a GCD concurrent queue before executing the callback code. Because GCD will create new threads if all existing threads are blocked (unlike swift concurrency) this effectively prevents deadlock risks at the risk of thread explosion. It would be better if the codebase was free of both the risks of deadlock and thread explosion. Additionally it would be nice if the APIs using `temp_await` were marked async so that more parts of the code base could safely adopt `async`/`await.` ### Modifications: * To augment existing memoizer types new async types are added `temp_dir` is replaced by `unsafe_await` which is limited to one used which is called by llbuild * `safe_async` is replaced by CheckedThrowingContinuation * Multiple protocols have been modified to change their requirements from sync to async * Usage of DispatchSemaphore and DispatchGroup have been replaced with structured concurrency * Many methods that were non-async but used `temp_await` have been properly marked as `async` ### Result: No usage of `temp_dir` or `safe_async` and only one (safe) usage of`unsafe_await` from an LLBuild command
1 parent b24cc8b commit 1764337

19 files changed

+599
-561
lines changed

‎Sources/Basics/Archiver/Archiver.swift‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,10 @@ public protocol Archiver: Sendable {
3636
/// - Parameters:
3737
/// - directory: The `AbsolutePath` to the archive to extract.
3838
/// - destinationPath: The `AbsolutePath` to the directory to extract to.
39-
/// - completion: The completion handler that will be called when the operation finishes to notify of its success.
40-
@available(*, noasync, message: "Use the async alternative")
4139
func compress(
4240
directory: AbsolutePath,
43-
to destinationPath: AbsolutePath,
44-
completion: @escaping @Sendable (Result<Void, Error>) -> Void
45-
)
41+
to destinationPath: AbsolutePath
42+
) async throws
4643

4744
/// Asynchronously validates if a file is an archive.
4845
///
@@ -71,20 +68,6 @@ extension Archiver {
7168
}
7269
}
7370

74-
/// Asynchronously compresses the contents of a directory to a destination archive.
75-
///
76-
/// - Parameters:
77-
/// - directory: The `AbsolutePath` to the archive to extract.
78-
/// - destinationPath: The `AbsolutePath` to the directory to extract to.
79-
public func compress(
80-
directory: AbsolutePath,
81-
to destinationPath: AbsolutePath
82-
) async throws {
83-
try await withCheckedThrowingContinuation { continuation in
84-
self.compress(directory: directory, to: destinationPath, completion: { continuation.resume(with: $0) })
85-
}
86-
}
87-
8871
/// Asynchronously validates if a file is an archive.
8972
///
9073
/// - Parameters:

‎Sources/Basics/Archiver/TarArchiver.swift‎

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -82,36 +82,29 @@ public struct TarArchiver: Archiver {
8282

8383
public func compress(
8484
directory: AbsolutePath,
85-
to destinationPath: AbsolutePath,
86-
completion: @escaping @Sendable (Result<Void, Error>) -> Void
87-
) {
88-
do {
89-
guard self.fileSystem.isDirectory(directory) else {
90-
throw FileSystemError(.notDirectory, directory.underlying)
91-
}
85+
to destinationPath: AbsolutePath
86+
) async throws {
9287

93-
let process = AsyncProcess(
94-
arguments: [self.tarCommand, "acf", destinationPath.pathString, directory.basename],
95-
environment: .current,
96-
workingDirectory: directory.parentDirectory
97-
)
88+
guard self.fileSystem.isDirectory(directory) else {
89+
throw FileSystemError(.notDirectory, directory.underlying)
90+
}
9891

99-
guard let registrationKey = self.cancellator.register(process) else {
100-
throw CancellationError.failedToRegisterProcess(process)
101-
}
92+
let process = AsyncProcess(
93+
arguments: [self.tarCommand, "acf", destinationPath.pathString, directory.basename],
94+
environment: .current,
95+
workingDirectory: directory.parentDirectory
96+
)
10297

103-
DispatchQueue.sharedConcurrent.async {
104-
defer { self.cancellator.deregister(registrationKey) }
105-
completion(.init(catching: {
106-
try process.launch()
107-
let processResult = try process.waitUntilExit()
108-
guard processResult.exitStatus == .terminated(code: 0) else {
109-
throw try StringError(processResult.utf8stderrOutput())
110-
}
111-
}))
112-
}
113-
} catch {
114-
return completion(.failure(error))
98+
guard let registrationKey = self.cancellator.register(process) else {
99+
throw CancellationError.failedToRegisterProcess(process)
100+
}
101+
102+
defer { self.cancellator.deregister(registrationKey) }
103+
104+
try process.launch()
105+
let processResult = try await process.waitUntilExit()
106+
guard processResult.exitStatus == .terminated(code: 0) else {
107+
throw try StringError(processResult.utf8stderrOutput())
115108
}
116109
}
117110

‎Sources/Basics/Archiver/UniversalArchiver.swift‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,10 @@ public struct UniversalArchiver: Archiver {
8585

8686
public func compress(
8787
directory: AbsolutePath,
88-
to destinationPath: AbsolutePath,
89-
completion: @escaping @Sendable (Result<Void, Swift.Error>) -> Void
90-
) {
91-
do {
92-
let archiver = try archiver(for: destinationPath)
93-
archiver.compress(directory: directory, to: destinationPath, completion: completion)
94-
} catch {
95-
completion(.failure(error))
96-
}
88+
to destinationPath: AbsolutePath
89+
) async throws {
90+
let archiver = try archiver(for: destinationPath)
91+
try await archiver.compress(directory: directory, to: destinationPath)
9792
}
9893

9994
public func validate(

‎Sources/Basics/Archiver/ZipArchiver.swift‎

Lines changed: 35 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -73,52 +73,44 @@ public struct ZipArchiver: Archiver, Cancellable {
7373

7474
public func compress(
7575
directory: AbsolutePath,
76-
to destinationPath: AbsolutePath,
77-
completion: @escaping @Sendable (Result<Void, Error>) -> Void
78-
) {
79-
do {
80-
guard self.fileSystem.isDirectory(directory) else {
81-
throw FileSystemError(.notDirectory, directory.underlying)
82-
}
76+
to destinationPath: AbsolutePath
77+
) async throws {
78+
guard self.fileSystem.isDirectory(directory) else {
79+
throw FileSystemError(.notDirectory, directory.underlying)
80+
}
8381

84-
#if os(Windows)
85-
let process = AsyncProcess(
86-
// FIXME: are these the right arguments?
87-
arguments: ["tar.exe", "-a", "-c", "-f", destinationPath.pathString, directory.basename],
88-
workingDirectory: directory.parentDirectory
89-
)
90-
#else
91-
// This is to work around `swift package-registry publish` tool failing on
92-
// Amazon Linux 2 due to it having an earlier Glibc version (rdar://116370323)
93-
// and therefore posix_spawn_file_actions_addchdir_np is unavailable.
94-
// Instead of passing `workingDirectory` param to TSC.Process, which will trigger
95-
// SPM_posix_spawn_file_actions_addchdir_np_supported check, we shell out and
96-
// do `cd` explicitly before `zip`.
97-
let process = AsyncProcess(
98-
arguments: [
99-
"/bin/sh",
100-
"-c",
101-
"cd \(directory.parentDirectory.underlying.pathString) && zip -r \(destinationPath.pathString) \(directory.basename)",
102-
]
103-
)
104-
#endif
82+
#if os(Windows)
83+
let process = AsyncProcess(
84+
// FIXME: are these the right arguments?
85+
arguments: ["tar.exe", "-a", "-c", "-f", destinationPath.pathString, directory.basename],
86+
workingDirectory: directory.parentDirectory
87+
)
88+
#else
89+
// This is to work around `swift package-registry publish` tool failing on
90+
// Amazon Linux 2 due to it having an earlier Glibc version (rdar://116370323)
91+
// and therefore posix_spawn_file_actions_addchdir_np is unavailable.
92+
// Instead of passing `workingDirectory` param to TSC.Process, which will trigger
93+
// SPM_posix_spawn_file_actions_addchdir_np_supported check, we shell out and
94+
// do `cd` explicitly before `zip`.
95+
let process = AsyncProcess(
96+
arguments: [
97+
"/bin/sh",
98+
"-c",
99+
"cd \(directory.parentDirectory.underlying.pathString) && zip -r \(destinationPath.pathString) \(directory.basename)",
100+
]
101+
)
102+
#endif
103+
104+
guard let registrationKey = self.cancellator.register(process) else {
105+
throw CancellationError.failedToRegisterProcess(process)
106+
}
105107

106-
guard let registrationKey = self.cancellator.register(process) else {
107-
throw CancellationError.failedToRegisterProcess(process)
108-
}
108+
defer { self.cancellator.deregister(registrationKey) }
109109

110-
DispatchQueue.sharedConcurrent.async {
111-
defer { self.cancellator.deregister(registrationKey) }
112-
completion(.init(catching: {
113-
try process.launch()
114-
let processResult = try process.waitUntilExit()
115-
guard processResult.exitStatus == .terminated(code: 0) else {
116-
throw try StringError(processResult.utf8stderrOutput())
117-
}
118-
}))
119-
}
120-
} catch {
121-
return completion(.failure(error))
110+
try process.launch()
111+
let processResult = try await process.waitUntilExit()
112+
guard processResult.exitStatus == .terminated(code: 0) else {
113+
throw try StringError(processResult.utf8stderrOutput())
122114
}
123115
}
124116

‎Sources/Basics/AuthorizationProvider.swift‎

Lines changed: 22 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -22,41 +22,14 @@ public protocol AuthorizationProvider: Sendable {
2222
}
2323

2424
public protocol AuthorizationWriter {
25-
@available(*, noasync, message: "Use the async alternative")
2625
func addOrUpdate(
2726
for url: URL,
2827
user: String,
2928
password: String,
30-
persist: Bool,
31-
callback: @escaping (Result<Void, Error>) -> Void
32-
)
29+
persist: Bool
30+
) async throws
3331

34-
@available(*, noasync, message: "Use the async alternative")
35-
func remove(for url: URL, callback: @escaping (Result<Void, Error>) -> Void)
36-
}
37-
38-
public extension AuthorizationWriter {
39-
func addOrUpdate(
40-
for url: URL,
41-
user: String,
42-
password: String,
43-
persist: Bool = true
44-
) async throws {
45-
try await safe_async {
46-
self.addOrUpdate(
47-
for: url,
48-
user: user,
49-
password: password,
50-
persist: persist,
51-
callback: $0)
52-
}
53-
}
54-
55-
func remove(for url: URL) async throws {
56-
try await safe_async {
57-
self.remove(for: url, callback: $0)
58-
}
59-
}
32+
func remove(for url: URL) async throws
6033
}
6134

6235
public enum AuthorizationProviderError: Error {
@@ -100,24 +73,23 @@ public final class NetrcAuthorizationProvider: AuthorizationProvider, Authorizat
10073
for url: URL,
10174
user: String,
10275
password: String,
103-
persist: Bool = true,
104-
callback: @escaping (Result<Void, Error>) -> Void
105-
) {
76+
persist: Bool = true
77+
) async throws {
10678
guard let machine = Self.machine(for: url) else {
107-
return callback(.failure(AuthorizationProviderError.invalidURLHost))
79+
throw AuthorizationProviderError.invalidURLHost
10880
}
10981

11082
if !persist {
11183
self.cache[machine] = (user, password)
112-
return callback(.success(()))
84+
return
11385
}
11486

11587
// Same entry already exists, no need to add or update
11688
let netrc = try? Self.load(fileSystem: self.fileSystem, path: self.path)
11789
guard netrc?.machines
11890
.first(where: { $0.name.lowercased() == machine && $0.login == user && $0.password == password }) == nil
11991
else {
120-
return callback(.success(()))
92+
return
12193
}
12294

12395
do {
@@ -134,21 +106,15 @@ public final class NetrcAuthorizationProvider: AuthorizationProvider, Authorizat
134106
stream.write("\n")
135107
}
136108
}
137-
138-
callback(.success(()))
139109
} catch {
140-
callback(.failure(
141-
AuthorizationProviderError
142-
.other("Failed to update netrc file at \(self.path): \(error.interpolationDescription)")
143-
))
110+
throw AuthorizationProviderError
111+
.other("Failed to update netrc file at \(self.path): \(error.interpolationDescription)")
144112
}
145113
}
146114

147-
public func remove(for url: URL, callback: @escaping (Result<Void, Error>) -> Void) {
148-
callback(.failure(
149-
AuthorizationProviderError
150-
.other("User must edit netrc file at \(self.path) manually to remove entries")
151-
))
115+
public func remove(for url: URL) async throws {
116+
throw AuthorizationProviderError
117+
.other("User must edit netrc file at \(self.path) manually to remove entries")
152118
}
153119

154120
public func authentication(for url: URL) -> (user: String, password: String)? {
@@ -217,47 +183,36 @@ public final class KeychainAuthorizationProvider: AuthorizationProvider, Authori
217183
for url: URL,
218184
user: String,
219185
password: String,
220-
persist: Bool = true,
221-
callback: @escaping (Result<Void, Error>) -> Void
222-
) {
186+
persist: Bool = true
187+
) async throws {
223188
guard let protocolHostPort = ProtocolHostPort(from: url) else {
224-
return callback(.failure(AuthorizationProviderError.invalidURLHost))
189+
throw AuthorizationProviderError.invalidURLHost
225190
}
226191

227192
self.observabilityScope
228193
.emit(debug: "add/update credentials for '\(protocolHostPort)' [\(url.absoluteString)] in keychain")
229194

230195
if !persist {
231196
self.cache[protocolHostPort.description] = (user, password)
232-
return callback(.success(()))
197+
return
233198
}
234199

235200
let passwordData = Data(password.utf8)
236201

237-
do {
238-
if !(try self.update(protocolHostPort: protocolHostPort, account: user, password: passwordData)) {
239-
try self.create(protocolHostPort: protocolHostPort, account: user, password: passwordData)
240-
}
241-
callback(.success(()))
242-
} catch {
243-
callback(.failure(error))
202+
if !(try self.update(protocolHostPort: protocolHostPort, account: user, password: passwordData)) {
203+
try self.create(protocolHostPort: protocolHostPort, account: user, password: passwordData)
244204
}
245205
}
246206

247-
public func remove(for url: URL, callback: @escaping (Result<Void, Error>) -> Void) {
207+
public func remove(for url: URL) async throws {
248208
guard let protocolHostPort = ProtocolHostPort(from: url) else {
249-
return callback(.failure(AuthorizationProviderError.invalidURLHost))
209+
throw AuthorizationProviderError.invalidURLHost
250210
}
251211

252212
self.observabilityScope
253213
.emit(debug: "remove credentials for '\(protocolHostPort)' [\(url.absoluteString)] from keychain")
254214

255-
do {
256-
try self.delete(protocolHostPort: protocolHostPort)
257-
callback(.success(()))
258-
} catch {
259-
callback(.failure(error))
260-
}
215+
try self.delete(protocolHostPort: protocolHostPort)
261216
}
262217

263218
public func authentication(for url: URL) -> (user: String, password: String)? {

0 commit comments

Comments
 (0)