Skip to content

Commit

Permalink
Fix Linux Process deadlocks (#67) (thanks @gregc !!!)
Browse files Browse the repository at this point in the history
* initial attempt at adding async/await to `Process`

* ParsableCommand -> AsyncParsableCommand

* oops, wasn't calling async main

* remove all waitUntilExit() remnants

* change static let gitURL to gregcotten/swift-bundler for testing

* more async/await

* don't use DispatchSemaphore which can deadlock

no clue if this works or fixes anything

* simplify getOutputData

* bring back fileHandleForReading.readToEnd()

* move fileHandleForReading.readToEnd to the data consuming Task

* add more getOutputData debugging

* more debug logging

* close inputPipe fileHandleForWriting after writing context?

* signal debug log

* readLine using async iterator if possible

* I guess only Darwin has AsyncBytes...

* readLineAsync always

* have readLineAsync respect `strippingNewline`

* builder subprocess input pipe should not be the parent's (?)

* flailing

* better builder debugging

* try using SwiftCommand in BuilderContextImpl

* shim in SwiftCommand in some flailing attempt to fix deadlock

* Update Package.swift

* Update ProjectBuilder.swift

* hardcode builder context as test

* try using Shwift in Builder

* Revert "try using Shwift in Builder"

This reverts commit a945c78.

* Revert "hardcode builder context as test"

This reverts commit b3ea18c.

* try calling "swift run Builder" instead of running exe directly?

* hardcoded context once again

* use AsyncProcess

* oops

* Update Package.resolved

* Update Package.resolved

* Update Package.resolved

* use tagged `AsyncProcess`

* remove extraneous logging

* Squashed commit of the following:

commit ea59e6e
Merge: 4cfdd0e 5bd25e0
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 11:48:59 2025 -0800

    Merge branch 'main' into async-process

commit 4cfdd0e
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 11:46:58 2025 -0800

    remove extraneous logging

commit b1f4f39
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 11:15:36 2025 -0800

    use tagged `AsyncProcess`

commit 80c4429
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 10:38:54 2025 -0800

    Update Package.resolved

commit 1243bed
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 10:26:24 2025 -0800

    Update Package.resolved

commit cbcbe46
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 09:52:30 2025 -0800

    Update Package.resolved

commit e68cd8d
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 09:42:04 2025 -0800

    oops

commit c1066c1
Author: Greg Cotten <[email protected]>
Date:   Wed Mar 5 09:41:34 2025 -0800

    use AsyncProcess

commit 2c171f7
Author: Greg Cotten <[email protected]>
Date:   Tue Mar 4 09:03:17 2025 -0800

    hardcoded context once again

commit ba4190c
Author: Greg Cotten <[email protected]>
Date:   Mon Mar 3 17:05:07 2025 -0800

    try calling "swift run Builder" instead of running exe directly?

commit 7b6d3e3
Author: Greg Cotten <[email protected]>
Date:   Mon Mar 3 14:58:08 2025 -0800

    Revert "hardcode builder context as test"

    This reverts commit b3ea18c.

commit 1062101
Author: Greg Cotten <[email protected]>
Date:   Mon Mar 3 14:58:00 2025 -0800

    Revert "try using Shwift in Builder"

    This reverts commit a945c78.

commit a945c78
Author: Greg Cotten <[email protected]>
Date:   Sun Mar 2 07:15:53 2025 -0800

    try using Shwift in Builder

commit b3ea18c
Author: Greg Cotten <[email protected]>
Date:   Sat Mar 1 07:09:46 2025 -0800

    hardcode builder context as test

commit 959f678
Author: Greg Cotten <[email protected]>
Date:   Fri Feb 28 13:23:17 2025 -0800

    Update ProjectBuilder.swift

commit 086f9f8
Author: Greg Cotten <[email protected]>
Date:   Fri Feb 28 13:22:23 2025 -0800

    Update Package.swift

commit d46600f
Author: Greg Cotten <[email protected]>
Date:   Fri Feb 28 13:21:12 2025 -0800

    shim in SwiftCommand in some flailing attempt to fix deadlock

commit f699a44
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 21:03:10 2025 -0800

    try using SwiftCommand in BuilderContextImpl

commit 065e3c0
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 19:55:42 2025 -0800

    better builder debugging

commit b6202f5
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 15:00:59 2025 -0800

    flailing

commit 65a0c7a
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 14:31:08 2025 -0800

    builder subprocess input pipe should not be the parent's (?)

commit 295bee1
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 14:16:28 2025 -0800

    have readLineAsync respect `strippingNewline`

commit 1bb3c00
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 14:06:19 2025 -0800

    readLineAsync always

commit 3360f60
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 14:03:41 2025 -0800

    I guess only Darwin has AsyncBytes...

commit 16fa1cb
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 13:52:35 2025 -0800

    readLine using async iterator if possible

commit 533fb18
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 12:53:26 2025 -0800

    signal debug log

commit 8b5f0fd
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 12:52:23 2025 -0800

    close inputPipe fileHandleForWriting after writing context?

commit cf242e7
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 12:36:30 2025 -0800

    more debug logging

commit 6e0b8fd
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 12:05:55 2025 -0800

    add more getOutputData debugging

commit 8957ddc
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 11:59:11 2025 -0800

    move fileHandleForReading.readToEnd to the data consuming Task

commit 0f2e019
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 11:50:33 2025 -0800

    bring back fileHandleForReading.readToEnd()

commit f6e8f81
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 11:45:21 2025 -0800

    simplify getOutputData

commit 14775ee
Author: Greg Cotten <[email protected]>
Date:   Thu Feb 27 10:01:51 2025 -0800

    don't use DispatchSemaphore which can deadlock

    no clue if this works or fixes anything

commit e9d9225
Author: Greg Cotten <[email protected]>
Date:   Wed Feb 26 22:29:49 2025 -0800

    more async/await

commit 4a0a3a5
Author: Greg Cotten <[email protected]>
Date:   Wed Feb 26 22:15:34 2025 -0800

    change static let gitURL to gregcotten/swift-bundler for testing

commit 25f773e
Author: Greg Cotten <[email protected]>
Date:   Wed Feb 26 21:50:33 2025 -0800

    remove all waitUntilExit() remnants

commit d5169d8
Author: Greg Cotten <[email protected]>
Date:   Wed Feb 26 20:38:10 2025 -0800

    oops, wasn't calling async main

commit 52a5bd2
Author: Greg Cotten <[email protected]>
Date:   Wed Feb 26 20:30:49 2025 -0800

    ParsableCommand -> AsyncParsableCommand

commit ecb5d62
Author: Greg Cotten <[email protected]>
Date:   Wed Feb 26 20:04:40 2025 -0800

    initial attempt at adding async/await to `Process`

* back to stackotter swift-bundler

* swift-format -> swift format

* oneshot swift format

* no need for swift-system import remnant

* get rid of swift 6 Result extension stuff

* format

* log `Process.run` everywhere possible

* address concerns

* formatting

* update AsyncProcess to 0.0.4

* AsyncProcess 0.0.5
  • Loading branch information
gregcotten authored Mar 6, 2025
1 parent 5bd25e0 commit 24f4323
Show file tree
Hide file tree
Showing 61 changed files with 863 additions and 515 deletions.
40 changes: 38 additions & 2 deletions Package.resolved
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,24 @@
"version" : "4.6.1"
}
},
{
"identity" : "async-collections",
"kind" : "remoteSourceControl",
"location" : "https://github.com/adam-fowler/async-collections.git",
"state" : {
"revision" : "726af96095a19df6b8053ddbaed0a727aa70ccb2",
"version" : "0.1.0"
}
},
{
"identity" : "asyncprocess",
"kind" : "remoteSourceControl",
"location" : "https://github.com/gregcotten/AsyncProcess",
"state" : {
"revision" : "3a506bb36e4aa54eb3f1b13f15e636c68ee3eb19",
"version" : "0.0.5"
}
},
{
"identity" : "jsonutilities",
"kind" : "remoteSourceControl",
Expand Down Expand Up @@ -77,8 +95,17 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-async-algorithms.git",
"state" : {
"revision" : "9cfed92b026c524674ed869a4ff2dcfdeedf8a2a",
"version" : "0.1.0"
"revision" : "4c3ea81f81f0a25d0470188459c6d4bf20cf2f97",
"version" : "1.0.3"
}
},
{
"identity" : "swift-atomics",
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-atomics.git",
"state" : {
"revision" : "cd142fd2f64be2100422d658e7411e39489da985",
"version" : "1.2.0"
}
},
{
Expand Down Expand Up @@ -162,6 +189,15 @@
"version" : "1.6.1"
}
},
{
"identity" : "swift-nio",
"kind" : "remoteSourceControl",
"location" : "https://github.com/apple/swift-nio.git",
"state" : {
"revision" : "c51907a839e63ebf0ba2076bba73dd96436bd1b9",
"version" : "2.81.0"
}
},
{
"identity" : "swift-overture",
"kind" : "remoteSourceControl",
Expand Down
18 changes: 16 additions & 2 deletions Package.swift
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,13 @@ let package = Package(
.package(url: "https://github.com/apple/swift-asn1", from: "1.1.0"),
.package(url: "https://github.com/apple/swift-crypto", from: "3.10.0"),
.package(url: "https://github.com/CoreOffice/XMLCoder", from: "0.17.1"),
.package(url: "https://github.com/adam-fowler/async-collections.git", from: "0.1.0"),
.package(url: "https://github.com/gregcotten/AsyncProcess", from: "0.0.5"),

// File watcher dependencies
.package(url: "https://github.com/sersoft-gmbh/swift-inotify", "0.4.0"..<"0.5.0"),
.package(url: "https://github.com/apple/swift-system", from: "1.2.0"),
.package(url: "https://github.com/apple/swift-async-algorithms", from: "0.1.0"),
.package(url: "https://github.com/apple/swift-async-algorithms", from: "1.0.3"),
],
targets: [
.executableTarget(
Expand All @@ -55,6 +57,12 @@ let package = Package(
.product(name: "SwiftSyntax", package: "swift-syntax"),
.product(name: "SwiftSyntaxBuilder", package: "swift-syntax"),
.product(name: "Overture", package: "swift-overture"),
.product(name: "AsyncCollections", package: "async-collections"),
.product(
name: "ProcessSpawnSync",
package: "AsyncProcess",
condition: .when(platforms: [.linux])
),

// Xcodeproj related dependencies
.product(
Expand Down Expand Up @@ -117,7 +125,13 @@ let package = Package(

.target(
name: "SwiftBundlerBuilders",
dependencies: []
dependencies: [
.product(
name: "ProcessSpawnSync",
package: "AsyncProcess",
condition: .when(platforms: [.linux])
)
]
),

.target(
Expand Down
23 changes: 18 additions & 5 deletions Plugins/SwiftBundlerCommandPlugin/SwiftBundlerCommandPlugin.swift
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,19 @@ import PackagePlugin
@main
struct SwiftBundlerCommandPlugin: CommandPlugin {
/// This entry point is called when operating on a Swift package.
func performCommand(context: PluginContext, arguments: [String]) throws {
func performCommand(context: PluginContext, arguments: [String]) async throws {
let bundler = try context.tool(named: "swift-bundler")

try run(command: bundler.path, with: arguments)
try await run(command: bundler.path, with: arguments)
}
}

extension SwiftBundlerCommandPlugin {
/// Run a command with the given arguments.
func run(command: Path, with arguments: [String]) throws {
func run(command: Path, with arguments: [String]) async throws {
let exec = URL(fileURLWithPath: command.string)

let process = try Process.run(exec, arguments: arguments)
process.waitUntilExit()
let process = try await Process.runAndWait(exec, arguments: arguments)

// Check whether the subprocess invocation was successful.
if process.terminationReason == .exit,
Expand All @@ -30,3 +29,17 @@ extension SwiftBundlerCommandPlugin {
}
}
}

extension Process {
class func runAndWait(_ url: URL, arguments: [String]) async throws -> Process {
try await withCheckedThrowingContinuation { c in

Check failure on line 35 in Plugins/SwiftBundlerCommandPlugin/SwiftBundlerCommandPlugin.swift

View workflow job for this annotation

GitHub Actions / swift-lint

Identifier Name Violation: Variable name 'c' should be between 3 and 60 characters long (identifier_name)
do {
_ = try Process.run(url, arguments: arguments) {
c.resume(returning: $0)
}
} catch {
c.resume(throwing: error)
}
}
}
}
38 changes: 32 additions & 6 deletions Sources/SwiftBundlerBuilders/Builder.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import Foundation
/// A project builder.
public protocol Builder {
/// Builds the project defined by the given context.
static func build(_ context: some BuilderContext) throws -> BuilderResult
static func build(_ context: some BuilderContext) async throws -> BuilderResult
}

private enum BuilderError: LocalizedError {
Expand All @@ -19,20 +19,46 @@ private enum BuilderError: LocalizedError {

extension Builder {
/// Default builder entrypoint. Parses builder context from stdin.
public static func main() {
public static func main() async {
do {
guard let input = readLine(strippingNewline: true) else {
throw BuilderError.noInput
}
let input = try await readLineAsync()

let context = try JSONDecoder().decode(
_BuilderContextImpl.self, from: Data(input.utf8)
)

_ = try build(context)
_ = try await build(context)
} catch {
print(error)
Foundation.exit(1)
}
}
}

func readLineAsync(strippingNewline: Bool = true) async throws -> String {
let readBytesStream = AsyncStream.makeStream(of: Data.self)

FileHandle.standardInput.readabilityHandler = { handle in
readBytesStream.continuation.yield(handle.availableData)
}

defer { FileHandle.standardInput.readabilityHandler = nil }

var accumulatedData = Data()
for await data in readBytesStream.stream {
accumulatedData.append(data)
if let stringData = String(data: accumulatedData, encoding: .utf8),
stringData.contains(where: { $0.isNewline }),
let firstLine = stringData.components(separatedBy: .newlines).first
{
readBytesStream.continuation.finish()
if strippingNewline {
return firstLine
} else {
return firstLine + "\n"
}
}
}

throw BuilderError.noInput
}
27 changes: 24 additions & 3 deletions Sources/SwiftBundlerBuilders/BuilderContextImpl.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import Foundation

#if os(Linux)
import ProcessSpawnSync
typealias Process = PSProcess
#endif

//swiftlint:disable type_name

Check warning on line 8 in Sources/SwiftBundlerBuilders/BuilderContextImpl.swift

View workflow job for this annotation

GitHub Actions / swift-lint

Comment Spacing Violation: Prefer at least one space after slashes for comments (comment_spacing)
// TODO: Use `package` access level when we bump to Swift 5.9

Check warning on line 9 in Sources/SwiftBundlerBuilders/BuilderContextImpl.swift

View workflow job for this annotation

GitHub Actions / swift-lint

Todo Violation: TODOs should be resolved (Use `package` access level whe...) (todo)
/// Implementation detail, may have breaking changes from time to time.
Expand All @@ -14,10 +19,11 @@ public struct _BuilderContextImpl: BuilderContext, Codable {
}

enum Error: LocalizedError {
case commandNotFound
case nonZeroExitStatus(Int)
}

public func run(_ command: String, _ arguments: [String]) throws {
public func run(_ command: String, _ arguments: [String]) async throws {
let process = Process()
#if os(Windows)
process.executableURL = URL(fileURLWithPath: "C:\\Windows\\System32\\cmd.exe")
Expand All @@ -27,8 +33,7 @@ public struct _BuilderContextImpl: BuilderContext, Codable {
process.arguments = [command] + arguments
#endif

try process.run()
process.waitUntilExit()
try await process.runAndWait()

let exitStatus = Int(process.terminationStatus)
guard exitStatus == 0 else {
Expand All @@ -37,3 +42,19 @@ public struct _BuilderContextImpl: BuilderContext, Codable {
}
}
//swiftlint:enable type_name

Check warning on line 44 in Sources/SwiftBundlerBuilders/BuilderContextImpl.swift

View workflow job for this annotation

GitHub Actions / swift-lint

Comment Spacing Violation: Prefer at least one space after slashes for comments (comment_spacing)

extension Process {
func runAndWait() async throws {
try await withCheckedThrowingContinuation { (continuation: CheckedContinuation<Void, Error>) in
terminationHandler = { _ in
continuation.resume()
}

do {
try run()
} catch {
continuation.resume(throwing: error)
}
}
}
}
2 changes: 1 addition & 1 deletion Sources/SwiftBundlerBuilders/Context.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ public protocol BuilderContext {

/// Runs the given command (either a path or a tool name) with the given
/// arguments.
func run(_ command: String, _ arguments: [String]) throws
func run(_ command: String, _ arguments: [String]) async throws
}
6 changes: 3 additions & 3 deletions Sources/swift-bundler/Bundler/AppImageBundler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@ enum AppImageBundler: Bundler {
static func bundle(
_ context: BundlerContext,
_ additionalContext: Context
) -> Result<BundlerOutputStructure, AppImageBundlerError> {
) async -> Result<BundlerOutputStructure, AppImageBundlerError> {
let outputStructure = intendedOutput(in: context, additionalContext)
let appDir = context.outputDirectory
.appendingPathComponent("\(context.appName).AppDir")
let bundleName = outputStructure.bundle.lastPathComponent

return GenericLinuxBundler.bundle(
return await GenericLinuxBundler.bundle(
context,
GenericLinuxBundler.Context(cosmeticBundleName: bundleName)
)
Expand Down Expand Up @@ -69,7 +69,7 @@ enum AppImageBundler: Bundler {
}
.andThenDoSideEffect { structure in
log.info("Converting '\(context.appName).AppDir' to '\(bundleName)'")
return AppImageTool.bundle(appDir: appDir, to: outputStructure.bundle)
return await AppImageTool.bundle(appDir: appDir, to: outputStructure.bundle)
.mapError { .failedToBundleAppDir($0) }
}
.replacingSuccessValue(with: outputStructure)
Expand Down
4 changes: 2 additions & 2 deletions Sources/swift-bundler/Bundler/AppImageTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import Foundation
/// A swifty interface for the `appimagetool` command-line tool for converting AppDirs to
/// AppImages.
enum AppImageTool {
static func bundle(appDir: URL, to appImage: URL) -> Result<Void, AppImageToolError> {
static func bundle(appDir: URL, to appImage: URL) async -> Result<Void, AppImageToolError> {
let arguments = [appDir.path, appImage.path]
return Process.runAppImage("appimagetool", arguments: arguments)
return await Process.runAppImage("appimagetool", arguments: arguments)
.mapError { error in
.failedToRunAppImageTool(
command: "appimagetool \(arguments.joined(separator: " "))",
Expand Down
4 changes: 2 additions & 2 deletions Sources/swift-bundler/Bundler/ArchiveTool.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,10 @@ enum ArchiveTool {
static func createTarGz(
of directory: URL,
at outputFile: URL
) -> Result<Void, ArchiveToolError> {
) async -> Result<Void, ArchiveToolError> {
let arguments = ["--create", "--file", outputFile.path, directory.lastPathComponent]
let workingDirectory = directory.deletingLastPathComponent()
return Process.create("tar", arguments: arguments, directory: workingDirectory)
return await Process.create("tar", arguments: arguments, directory: workingDirectory)
.runAndWait()
.mapError { error in
.failedToCreateTarGz(directory: directory, outputFile: outputFile, error)
Expand Down
2 changes: 1 addition & 1 deletion Sources/swift-bundler/Bundler/Bundler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ protocol Bundler {
static func bundle(
_ context: BundlerContext,
_ additionalContext: Context
) -> Result<BundlerOutputStructure, Error>
) async -> Result<BundlerOutputStructure, Error>

/// Returns a description of the files that would be produced if
/// ``Bundler/bundle(_:_:)`` were to get called with the provided context.
Expand Down
Loading

0 comments on commit 24f4323

Please sign in to comment.