CODEWITHSUNDEEP
mavenjenkinsfindbugs
Brian
Brian

Reputation: 13593

Is this a bug of findbugs?

I have a jenkins plugin which contains code like this:

public int getLastBuildNumber() {
    if (project != null && project.getLastBuild() != null ) {
        return project.getLastBuild().getNumber();
    }
    return 0;
}

When I publish the code with mvn release:prepare release:perform -Dusername=myusername -Dpassword=mypassword, I got a bug saying Possible null pointer dereference.

Here's the result of mvn findbugs:gui:

enter image description here

Why does it say The return value from a method is dereferenced without a null check?

I think project != null && project.getLastBuild() != null is the null check, isn't it?

Is this a bug of findbugs? How can I solve it? Or can I disable findbugs when releasing my jenkins plugin?

You can visit here for the full code.

Upvotes: 2

Views: 165

Answers (1)

Costi Ciudatu
Costi Ciudatu

Reputation: 38265

Unless your Project class is immutable or (at least) the value returned by getLastBuild() is a final field, your null-checks are not enough.

That's because, in theory, some other thread may call project.setLastBuild(null) just before your return statement:

if (project != null && project.getLastBuild() != null) {
    // thread T does project.setLastBuild(null) here -- for whatever reason
    return project.getLastBuild().getNumber(); // NPE!
}

To make this bullet proof, you need to move away from those moving parts and make your own local copies (synchronization is hardly an option in this case, imho):

LastBuild lastBuild = project != null ? project.getLastBuild() : null; // this only works as long as your project reference is final
return lastBuild != null ? lastBuild.getNumber() : 0;

Or, even better, make use of the Java8 Optional to perform all the necessary null-checks (implicitly) in one go:

return Optional.ofNullable(project)
    .map(Project::getLastBuild)
    .map(LastBuild::getNumber)
    .orElse(0);

Upvotes: 3

Related Questions