Process: Implement signal sending functions. #1578
Conversation
|
@swift-ci please test |
Foundation/Process.swift
Outdated
| suspendCount += 1 | ||
| if suspendCount == 1, processIdentifier > 0 { | ||
| kill(processIdentifier, SIGSTOP) | ||
| } |
There was a problem hiding this comment.
On Darwin we actually check the result of kill (for 0) before incrementing the count. Same for resume.
| case "--signal-test": signalTest() | ||
| default: break | ||
| } | ||
| } |
There was a problem hiding this comment.
I think we're going to have to rename this helper tool at some point :)
(no need to do it now)
There was a problem hiding this comment.
Does this logic look correct:
open func suspend() -> Bool {
guard isRunning && processIdentifier > 0 else {
return false
}
if suspendCount == 0 {
if kill(processIdentifier, SIGSTOP) == 0 {
suspendCount += 1
}
} else {
suspendCount += 1
}
return true
}
open func resume() -> Bool {
guard isRunning && processIdentifier > 0 else {
return true
}
if suspendCount == 1 {
if kill(processIdentifier, SIGCONT) == 0 {
suspendCount -= 1
}
} else {
suspendCount -= 1
}
return true
}
The tests ran OK on Linux/macOS and Darwin native so I think the counting is correct.
There was a problem hiding this comment.
Here is the code from Darwin, simplified slightly:
- (BOOL)suspend {
BOOL success = YES;
success = 0 == kill(_pid, SIGSTOP);
if (success) {
_suspendCount++;
}
return success;
}
- (BOOL)resume {
BOOL success = YES;
if (1 == _suspendCount) {
success = 0 == kill(_pid, SIGCONT);
}
if (success) {
_suspendCount--;
}
return success;
}
It doesn't look like we bother checking the pid, or isRunning. Those might be good improvements but I suspect if we were to change them on Darwin we would find someone who relies upon being able to send these before launching the app in some way.
|
@swift-ci please test |
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
@spevans I did some mangling around arguments in xdgTestHelper; can you rebase the patch on master once my FileManager patch finishes merging? |
spevans
force-pushed
the
pr_process_signals
branch
2 times, most recently
from
66432ca to
823c5da
Compare
|
@swift-ci please test |
spevans
force-pushed
the
pr_process_signals
branch
from
823c5da to
0940c2c
Compare
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
- Implements suspend(), resume(), interrupt() and terminate(). - Add --signal-test to xdgTestHelper for testing signals.
- .processIdentifier is 0 before launch and retains the PID value after termination. - .terminationStatus, .terminationReason: Add precondition that the task has run and exited. - interrupt(), termintate(): Add precondition that the task has started. - suspend(), resume(): Remove checks that process is running and match Darwin's suspendCount increment/decrement.
spevans
force-pushed
the
pr_process_signals
branch
from
0940c2c to
12c1638
Compare
|
@swift-ci please test |
1 similar comment
|
@swift-ci please test |
|
@parkera This should match the behaviour of Darwin more closely now. |
|
@swift-ci please test |
|
@swift-ci please test and merge |
Implements suspend(), resume(), interrupt() and terminate().
Add --signal-test to xdgTestHelper for testing signals.