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

Slave#createLauncher relies on autoboxing and can thrown NPE

      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.

          [JENKINS-38527] Slave#createLauncher relies on autoboxing and can thrown NPE

          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 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 added a comment -

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

          Oleg Nenashev added a comment - We hit it in another logic branch. I will fix it

          Oleg Nenashev added a comment -

          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/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 added a comment -

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

          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/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)

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

              Created:
              Updated:
              Resolved: