Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

Process.asyncExecute has threading issue #176

Open
weissi opened this issue Oct 15, 2018 · 0 comments
Open

Process.asyncExecute has threading issue #176

weissi opened this issue Oct 15, 2018 · 0 comments
Assignees
Labels
Milestone

Comments

@weissi
Copy link

weissi commented Oct 15, 2018

Process.asyncExecute contains this code:

var running = true
// readabilityHandler doesn't work on linux, so we are left with this hack
DispatchQueue.global().async {
while running {
let stdout = stdout.fileHandleForReading.availableData
if !stdout.isEmpty {
output(.stdout(stdout))
}
}
}
DispatchQueue.global().async {
while running {
let stderr = stderr.fileHandleForReading.availableData
if !stderr.isEmpty {
output(.stderr(stderr))
}
}
}

In particular, there's an unsynchronised, shared variable running that's read and written to from multiple threads. That is undefined behaviour which might amongst random crashes, infinite while running loops (even if running gets set to false) might lead to arbitrary worse behaviour.

This can easily be fixed by using a lock around every use of running. But I'd recommend restructuring the code. At present it keeps three background threads blocked for every execution of a process. Worse, because Dispatch normally uses a finitely sized thread-pool this code might just randomly deadlock. The reason it might randomly deadlock is that it will only finish when this code is executed:

DispatchQueue.global().async {
let process = launchProcess(path: program, arguments, stdout: stdout, stderr: stderr)
process.waitUntilExit()
running = false
promise.succeed(result: process.terminationStatus)

But the promise.succeed is inside of a globalQueue.async which needs an available dispatch thread to run. And while it waits for that dispatch thread to become available it blocks to others (to read stderr and stdout). In other words: We need three available dispatch threads to make progress which is an anti-pattern in Dispatch because the thread pool has a finite number of threads so it can totally happen that all of Dispatch's threads are blocked and only become unblocked when more threads become available (which won't happen if the thread pool is exhausted).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

3 participants