Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-38527

Slave#createLauncher relies on autoboxing and can thrown NPE

    XMLWordPrintable

Details

    Description

      in hudson.model.Slave#createLauncher
      hudson.Launcher.RemoteLauncher#RemoteLauncher(TaskListener,VirtualChannel,boolean) is invoked using Computer.isUnix() as argument, which return a Boolean. Autoboxing let us do such thing, but can result in NPE.

      Attachments

        Issue Links

          Activity

            oleg_nenashev Oleg Nenashev added a comment -

            Not a real defect since IsUnix command does not return null. But it still makes sense to fix that.

            public Launcher createLauncher(TaskListener listener) throws IOException, InterruptedException {
                    if(channel==null)
                        return new LocalLauncher(listener);
                    else
                        return new RemoteLauncher(listener,channel,channel.call(new IsUnix()));
                }
            
                private static final class IsUnix extends MasterToSlaveCallable<Boolean,IOException> {
                    public Boolean call() throws IOException {
                        return File.pathSeparatorChar==':';
                    }
                    private static final long serialVersionUID = 1L;
                }
            
            oleg_nenashev Oleg Nenashev added a comment - Not a real defect since IsUnix command does not return null. But it still makes sense to fix that. public Launcher createLauncher(TaskListener listener) throws IOException, InterruptedException { if(channel==null) return new LocalLauncher(listener); else return new RemoteLauncher(listener,channel,channel.call(new IsUnix())); } private static final class IsUnix extends MasterToSlaveCallable<Boolean,IOException> { public Boolean call() throws IOException { return File.pathSeparatorChar==':'; } private static final long serialVersionUID = 1L; }
            oleg_nenashev Oleg Nenashev added a comment -

            We hit it in another logic branch. I will fix it

            oleg_nenashev Oleg Nenashev added a comment - We hit it in another logic branch. I will fix it
            oleg_nenashev Oleg Nenashev added a comment - https://github.com/jenkinsci/jenkins/pull/2923

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            core/src/main/java/hudson/model/Slave.java
            http://jenkins-ci.org/commit/jenkins/78a42d5a4a5d545324c2d3230de6947e1ec8806e
            Log:
            JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics (#2923)

            • JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics

            The original issue comes from the isUnix() unboxing, but we can also get into an issue later if we pass a null Channel instance to the logic.
            This change adds some diagnostics which discovers potential root causes of such potential NPEs due to the race conditions with Computer reconnection

            • JENKINS-38527 - Also handle cases when Channel#isClosingOrClosed() as @stephenc suggested
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/java/hudson/model/Slave.java http://jenkins-ci.org/commit/jenkins/78a42d5a4a5d545324c2d3230de6947e1ec8806e Log: JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics (#2923) JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics The original issue comes from the isUnix() unboxing, but we can also get into an issue later if we pass a null Channel instance to the logic. This change adds some diagnostics which discovers potential root causes of such potential NPEs due to the race conditions with Computer reconnection JENKINS-38527 - Also handle cases when Channel#isClosingOrClosed() as @stephenc suggested
            oleg_nenashev Oleg Nenashev added a comment -

            The pull request has been merged towards 2.68. Marking as LTS candidate

            oleg_nenashev Oleg Nenashev added a comment - The pull request has been merged towards 2.68. Marking as LTS candidate

            Code changed in jenkins
            User: Oleg Nenashev
            Path:
            core/src/main/java/hudson/model/Slave.java
            http://jenkins-ci.org/commit/jenkins/c2163c5b3cb4a0c42cdc31cf404a82bcecb05082
            Log:
            JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics (#2923)

            • JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics

            The original issue comes from the isUnix() unboxing, but we can also get into an issue later if we pass a null Channel instance to the logic.
            This change adds some diagnostics which discovers potential root causes of such potential NPEs due to the race conditions with Computer reconnection

            • JENKINS-38527 - Also handle cases when Channel#isClosingOrClosed() as @stephenc suggested

            (cherry picked from commit 78a42d5a4a5d545324c2d3230de6947e1ec8806e)

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Oleg Nenashev Path: core/src/main/java/hudson/model/Slave.java http://jenkins-ci.org/commit/jenkins/c2163c5b3cb4a0c42cdc31cf404a82bcecb05082 Log: JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics (#2923) JENKINS-38527 - Prevent NullPointerException in Slave#createLauncher() and add cause diagnostics The original issue comes from the isUnix() unboxing, but we can also get into an issue later if we pass a null Channel instance to the logic. This change adds some diagnostics which discovers potential root causes of such potential NPEs due to the race conditions with Computer reconnection JENKINS-38527 - Also handle cases when Channel#isClosingOrClosed() as @stephenc suggested (cherry picked from commit 78a42d5a4a5d545324c2d3230de6947e1ec8806e)

            People

              oleg_nenashev Oleg Nenashev
              ndeloof Nicolas De Loof
              Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: