[JENKINS-72833] Do not attempt to self-exec on systems without libc
#9025
Conversation
| @@ -120,13 +120,13 @@ public void verifyRestartable() throws RestartNotSupportedException { | |||
| // if run on Unix, we can do restart | |||
| try { | |||
| instance = new UnixLifecycle(); | |||
| } catch (final IOException e) { | |||
| LOGGER.log(Level.WARNING, "Failed to install embedded lifecycle implementation", e); | |||
| } catch (final Throwable t) { | |||
There was a problem hiding this comment.
Dead code, since this constructor could never in fact throw IOException, but I thought it best to keep this fallback since any unanticipated errors here should not be fatal.
| @@ -50,16 +52,12 @@ | |||
| * @since 1.304 | |||
| */ | |||
| public class UnixLifecycle extends Lifecycle { | |||
|
|
|||
| @NonNull | |||
There was a problem hiding this comment.
This can never be null, so annotate it as such.
|
|
||
| public UnixLifecycle() throws IOException { |
There was a problem hiding this comment.
This can never throw IOException, so stop declaring it.
| try { | ||
| args = JavaVMArguments.current(); | ||
| } catch (UnsupportedOperationException | LinkageError e) { | ||
| // can't restart / see JENKINS-3875 | ||
| failedToObtainArgs = e; | ||
| } |
There was a problem hiding this comment.
This was based on the old implementation, which used Akuma. That was ripped out a long time ago, and these exceptions shouldn't be thrown in the current implementation. But just to be on the safe side, we're now catching Throwable in the caller, so this shouldn't result in a regression in any case.
| if (!Functions.isGlibcSupported()) { | ||
| throw new RestartNotSupportedException("Restart is not supported on platforms without libc"); | ||
| } |
There was a problem hiding this comment.
The fix for JENKINS-72833: do not attempt to use UnixLifecycle if JNA can't make calls to libc.
| if (args == null) | ||
| throw new RestartNotSupportedException("Failed to obtain the command line arguments of the process", failedToObtainArgs); |
There was a problem hiding this comment.
Dead code, since args can never be null.
| @@ -30,7 +31,7 @@ public static List<String> current() { | |||
| return args; | |||
| } else if (Functions.isGlibcSupported()) { | |||
| // Native approach | |||
| int pid = Ints.checkedCast(ProcessHandle.current().pid()); | |||
| int pid = Math.toIntExact(ProcessHandle.current().pid()); | |||
There was a problem hiding this comment.
/label ready-for-merge
This PR is now ready for merge. We will merge it after ~24 hours if there is no negative feedback.
Please see the merge process documentation for more information about the merge process.
Thanks!
comment-ops-bot
bot
added
the
ready-for-merge
See JENKINS-72833. We were attempting to use
UnixLifecycleon a system not supported by JNA.While I was here I also did some other cleanup of this class.
Testing done
Tested
/safeRestarton Linux both inside and outsidesystemd.Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Desired reviewers
@mention
Before the changes are marked as
ready-for-merge:Maintainer checklist