Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Linux Process deadlocks #67

Merged
merged 52 commits into from
Mar 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
ecb5d62
initial attempt at adding async/await to `Process`
gregcotten Feb 27, 2025
52a5bd2
ParsableCommand -> AsyncParsableCommand
gregcotten Feb 27, 2025
d5169d8
oops, wasn't calling async main
gregcotten Feb 27, 2025
25f773e
remove all waitUntilExit() remnants
gregcotten Feb 27, 2025
4a0a3a5
change static let gitURL to gregcotten/swift-bundler for testing
gregcotten Feb 27, 2025
e9d9225
more async/await
gregcotten Feb 27, 2025
14775ee
don't use DispatchSemaphore which can deadlock
gregcotten Feb 27, 2025
f6e8f81
simplify getOutputData
gregcotten Feb 27, 2025
0f2e019
bring back fileHandleForReading.readToEnd()
gregcotten Feb 27, 2025
8957ddc
move fileHandleForReading.readToEnd to the data consuming Task
gregcotten Feb 27, 2025
6e0b8fd
add more getOutputData debugging
gregcotten Feb 27, 2025
cf242e7
more debug logging
gregcotten Feb 27, 2025
8b5f0fd
close inputPipe fileHandleForWriting after writing context?
gregcotten Feb 27, 2025
533fb18
signal debug log
gregcotten Feb 27, 2025
16fa1cb
readLine using async iterator if possible
gregcotten Feb 27, 2025
3360f60
I guess only Darwin has AsyncBytes...
gregcotten Feb 27, 2025
1bb3c00
readLineAsync always
gregcotten Feb 27, 2025
295bee1
have readLineAsync respect `strippingNewline`
gregcotten Feb 27, 2025
65a0c7a
builder subprocess input pipe should not be the parent's (?)
gregcotten Feb 27, 2025
b6202f5
flailing
gregcotten Feb 27, 2025
065e3c0
better builder debugging
gregcotten Feb 28, 2025
f699a44
try using SwiftCommand in BuilderContextImpl
gregcotten Feb 28, 2025
d46600f
shim in SwiftCommand in some flailing attempt to fix deadlock
gregcotten Feb 28, 2025
086f9f8
Update Package.swift
gregcotten Feb 28, 2025
959f678
Update ProjectBuilder.swift
gregcotten Feb 28, 2025
b3ea18c
hardcode builder context as test
gregcotten Mar 1, 2025
a945c78
try using Shwift in Builder
gregcotten Mar 2, 2025
1062101
Revert "try using Shwift in Builder"
gregcotten Mar 3, 2025
7b6d3e3
Revert "hardcode builder context as test"
gregcotten Mar 3, 2025
ba4190c
try calling "swift run Builder" instead of running exe directly?
gregcotten Mar 4, 2025
2c171f7
hardcoded context once again
gregcotten Mar 4, 2025
c1066c1
use AsyncProcess
gregcotten Mar 5, 2025
e68cd8d
oops
gregcotten Mar 5, 2025
cbcbe46
Update Package.resolved
gregcotten Mar 5, 2025
1243bed
Update Package.resolved
gregcotten Mar 5, 2025
80c4429
Update Package.resolved
gregcotten Mar 5, 2025
b1f4f39
use tagged `AsyncProcess`
gregcotten Mar 5, 2025
4cfdd0e
remove extraneous logging
gregcotten Mar 5, 2025
ea59e6e
Merge branch 'main' into async-process
gregcotten Mar 5, 2025
0f2befa
Squashed commit of the following:
gregcotten Mar 5, 2025
ccf2914
back to stackotter swift-bundler
gregcotten Mar 5, 2025
e31bbb6
swift-format -> swift format
gregcotten Mar 5, 2025
8eefa51
oneshot swift format
gregcotten Mar 5, 2025
69f5d32
no need for swift-system import remnant
gregcotten Mar 5, 2025
f9de90b
get rid of swift 6 Result extension stuff
gregcotten Mar 6, 2025
ce28ded
format
gregcotten Mar 6, 2025
1b478ed
Merge branch 'async-process' into fix-linux-process
gregcotten Mar 6, 2025
29f4024
log `Process.run` everywhere possible
gregcotten Mar 6, 2025
4e7d93d
address concerns
gregcotten Mar 6, 2025
a764eba
formatting
gregcotten Mar 6, 2025
499fdf5
update AsyncProcess to 0.0.4
gregcotten Mar 6, 2025
1f5c6eb
AsyncProcess 0.0.5
gregcotten Mar 6, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
@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 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,7 +1,12 @@
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.
/// "Hidden" from users to avoid exposing implementation details such as
/// the ``Codable`` conformance, since the builder API has to be pretty
Expand All @@ -14,10 +19,11 @@
}

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 @@
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 @@ -36,4 +41,20 @@
}
}
}
//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
Loading