-
Notifications
You must be signed in to change notification settings - Fork 92
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
Try to be robust against LinkageError’s from JNA #47
Conversation
@@ -96,12 +96,16 @@ private static boolean _isAlive(VirtualChannel channel, int pid, Launcher launch | |||
this.pid = pid; | |||
} | |||
@Override public Boolean call() throws RuntimeException { | |||
// JNR-POSIX does not seem to work on FreeBSD at least, so using JNA instead. |
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.
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
|
Cannot reproduce such a failure locally, and CI server is timing out. |
I think this is caused by there being a process with PID=9999 on the test machine. Looking to see if there is some better way to figure this out. |
…at PID 9999 actually exists.
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.
It probably collides with other ProcessLiveness
and JNR PRs for Cygwin, but seems fine. 🐝
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.
But I would like that you explain why you know you need to check EXACTLY the port 9999. I don't get why you know that this is the PID to start looking.
We do not. It is just a highish PID number that is unlikely to be in use on a machine which has not done much yet. But it might be, hence the second commit here. We cannot just pick a really high number since the kernel will not accept values outside some range, IIRC 16k or so. |
@reviewbybees done |
@jglick I would like this to be merged and released :-) |
No longer necessary. |
An installation encountering kohsuke/akuma#15 which would normally be swallowed here nonetheless had trouble with warnings printed during Pipeline builds:
Seems we were not catching
LinkageError
s inProcessLiveness
correctly, and throwing them up intoexitStatus
.@reviewbybees