timja / jenkins-gh-issues-poc-06-18

0 stars 0 forks source link

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

Closed timja closed 7 years ago

timja commented 8 years ago

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.


Originally reported by ndeloof, imported from: Slave#createLauncher relies on autoboxing and can thrown NPE
  • assignee: oleg_nenashev
  • status: Resolved
  • priority: Minor
  • resolution: Fixed
  • resolved: 2017-06-26T09:03:32+00:00
  • imported: 2022/01/10
timja commented 7 years ago

oleg_nenashev:

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 {
public Boolean call() throws IOException {
    return File.pathSeparatorChar==':';
}
private static final long serialVersionUID = 1L;
    }
timja commented 7 years ago

oleg_nenashev:

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

timja commented 7 years ago

oleg_nenashev:

https://github.com/jenkinsci/jenkins/pull/2923

timja commented 7 years ago

scm_issue_link:

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)

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

timja commented 7 years ago

oleg_nenashev:

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

timja commented 7 years ago

scm_issue_link:

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)

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

(cherry picked from commit 78a42d5a4a5d545324c2d3230de6947e1ec8806e)

timja commented 2 years ago

[Originally duplicated by: JENKINS-23305]