-
Notifications
You must be signed in to change notification settings - Fork 124
Fix hang in ServiceConsoleTests.serviceShutdown #673
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 hang in ServiceConsoleTests.serviceShutdown #673
Conversation
@swift-ci test |
8677081
to
8a8a566
Compare
@swift-ci test |
8a8a566
to
d59ebe0
Compare
@swift-ci test |
48448c2
to
c95aeae
Compare
@swift-ci test |
c95aeae
to
996bcb1
Compare
This hang occurred only in CI environments and only on Linux. Here's the sequence of events: - Test terminates swbuild using SIGKILL - OS reparents SWBBuildService (a subprocess of swbuild) to launchd (Darwin) / init (others) - OS closes the file descriptors for the I/O pipes swbuild has connected to SWBBuildService - SWBBuildService's read() loop indicates EOF due to the broken pipe - SWBBuildService causes itself to exit At this point, the getpgid loop should return ERSCH and terminate the test. However, SWBBuildService is sticking around as a zombie for an extended period of time without init reaping the pid, causing getpgid to never hit the termination state. This causes the test to hang indefinitely. To fix this, there are two aspects: - A timeout is added around the termination monitoring loop that forces the exit promise to be fulfilled with an error if a 30-second interval elapses without the process exiting - We switch from using a getpgid loop to using a waitid loop, where the terminal state is that the process has _exited_... we don't care if the zombie hasn't been collected by init, only that it's not in a running state This fixes the hang for both the Jenkins based CI as well as GitHub actions, and also insulates us against future hangs by ensuring the test will terminate with a timeout error instead of hanging indefinitely, so that we at least know _which_ test is the problem.
996bcb1
to
d852b97
Compare
@swift-ci test |
import SystemPackage | ||
#endif | ||
|
||
@Suite(.skipHostOS(.windows)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: Will this cause the entire suite to be skipped on Windows, including individual test functions that aren't marked as skip for Windows?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, but note that this isn't new code. Many of these were broken on Windows which is why they were skipped en masse. We should work on getting them passing.
Some of these were formerly skipped in GitHub actions, but are passing now. Likely the culprit was ServiceConsoleTests.serviceShutdown all along, which is fixed in swiftlang#673
Some of these were formerly skipped in GitHub actions, but are passing now. Likely the culprit was ServiceConsoleTests.serviceShutdown all along, which is fixed in #673
This hang occurred only in CI environments and only on Linux. Here's the sequence of events:
At this point, the getpgid loop should return ERSCH and terminate the test. However, SWBBuildService is sticking around as a zombie for an extended period of time without init reaping the pid, causing getpgid to never hit the termination state. This causes the test to hang indefinitely.
To fix this, there are two aspects:
This fixes the hang for both the Jenkins based CI as well as GitHub actions, and also insulates us against future hangs by ensuring the test will terminate with a timeout error instead of hanging indefinitely, so that we at least know which test is the problem.