GlassFish - World's first Java EE 7 Application Server


FindBugs on GlassFish

Background

We're using FindBugs with GlassFish to, well, find bugs in GlassFish code, and generally to improve the quality of GlassFish. For GlassFish 4.0 we considered both the high and low priority errors reported by FindBugs, but we only checked the main GlassFish source workspace and a few others. Going forward, we'll consider more source workspaces.

How To

FindBugs is run on a regular basis and errors found are sent to the person who last touched the line with the error. Errors are sent to your java.net user ID; please make sure such messages are properly forwarded to your real email account, and that your mail filters don't hide such messages.

In addition, you can run FindBugs yourself. To run FindBugs on a project, use

% mvn findbugs:findbugs

This will create a target/findbugsXml.xml with the errors (if any). Once you have generated the file you can view the errors reported in it using the FindBugs GUI:

% mvn findbugs:gui

Alternatively, you can generate a site report for the project:

% mvn site:site site:staging

(Note that "mvn site:site" doesn't currently work correctly on the GlassFish workspace.)

You would view the FindBugs report as part of the site report in target/staging.

To make this work, you need a <distributionManagement> configuration in your pom.xml; see below.

Hudson Jobs

Several Hudson jobs run FindBugs on a regular basis. Unfortunately, these jobs are only visible inside Oracle. More jobs will be added; see below.

Post-4.0 Changes

We're going to expand our use of FindBugs in several ways.

Zero High Priority Bugs

We're going to continue our policy of enforcing zero high priority bugs in any of our workspaces, and we're going to continue to ensure we stay there by enforcing this as part of the nightly build. If any high priority errors are introduced, the nightly build will fail. This applies to the main GlassFish workspace as well as the workspaces listed below.

No New Errors

In addition to no high priority errors ever, we're going to enforce that the number of errors in each module in the GlassFish workspace may not increase. Ideally, we would enforce no new errors at all, and you can think of it that way if you prefer, but the actual rule allows some flexibility and some tradeoffs.

More Workspaces

In addition to the main GlassFish workspace, FindBugs will now be run on the top 15 Oracle-led workspaces that contribute code to the GlassFish project:

Workspace
mq
metro
mojarra
glassfish-corba
grizzly
jersey
JAXB
hk2
JavaMail
shoal
tyrus
jsp
JSTL
uel
gmbal

Most of those workspaces are in pretty good shape with the high priority FindBugs errors. As they get down to zero, their nightly builds will enforce that.

All Bugs

FindBugs defines three bug priorities - high, normal, and low. These priorities are really the bug confidence, that is, how likely it is that the bug pattern is being detected correctly. While we're only enforcing that there be no high priority bugs, we're looking at bugs of all priorities.

To run FindBugs and report on all bugs of all priorities, use this:

% mvn -Dfindbugs.threshold=Low findbugs:findbugs
New Bug Detectors

We've created some special purpose GlassFish-specific bug detectors to detect errors related to logging. Most of these new detectors will be included in our low priority jobs. These detectors report violations of the conventions described in the Logging Guide.

There are a large number of errors detected by the GF_INVALID_MSG_ID_PATTERN and GF_MISSING_LOGMESSAGE_INFO_ANNOTATION detectors. These errors will be tracked and managed separately and are not currently part of our overall goals described below.

A Note on FindBugs Priorities

As mentioned above, the FindBugs priority is really the confidence that a bug pattern is detected correctly. Detecting a bug pattern correctly doesn't necessarily mean that the impact of the bug is severe. To address this, the latest version of FindBugs introduces a bug rank. A low bug rank means that the bug, if it's detected correctly, and if it were to occur, would be very severe. A large bug rank number means the impact of the bug is minor.

Bug ranks are from 1 to 20.

The mapping between bug pattern and bug rank can also be customized, although we have no current plans to do so.

Exclude List

As you know, FindBugs supports an exclude list. There's about 8 high priority bugs on the exclude list. With medium and low priority bugs, a lot more bugs will be on the exclude list. In particular, I've already excluded these bugs:

This is a new FindBugs 2.0 error that complains about the use of the default character encoding. When reading and writing files on the local system, use of the default encoding is often appropriate. When sending data over the network, using the default is almost never appropriate. We have many cases of the former as well as some of the latter, but because so many of these errors are not real problems I've excluded this bug.

Turning anonymous inner classes into static inner classes is generally not worth the effort and usually makes the code less readable.

There's lots of places where we purposely catch and ignore exceptions. Probably some of them should be fixed, but there are too many to consider right now.

Likewise, there's lots of places where we catch exception "just in case" anything goes wrong. Probably most of them should catch RuntimeException or a smaller set of exceptions. But this doesn't seem important and there's too many to fix right now.

Lots of internal classes pass arrays in and out of methods, knowing that the caller is not going to modify the array. Yes, this is a risk with public APIs where we have no control over the caller, but there are too many of these errors to consider right now.

Yes, returning a zero length array is often a better design. But it's not an important issue.

Lots of classes extend Exception or Event classes and yet are never serialized so it doesn't really matter that they don't contain a serialVersionUID field.

  • EJB CMP code. Sigh.

Lots of errors in the EJB CMP code. We really don't want to touch it.

  • The verifier. Double sigh.

Lots and lots of errors in the verifier. We can't afford to deal with all these issues now.

Configuring Your Own Project To Use FindBugs

To configure your own project to use FindBugs, add the following entries to your main pom.xml. First, some property settings:

    <properties>
        <findbugs.skip>false</findbugs.skip>
        <findbugs.threshold>High</findbugs.threshold>
        <findbugs.common>exclude-common.xml</findbugs.common>
        <findbugs.exclude></findbugs.exclude>
        <findbugs.glassfish.logging.validLoggerPrefixes>
            javax.enterprise
        </findbugs.glassfish.logging.validLoggerPrefixes>
        <findbugs.version>2.5.2</findbugs.version>
    </properties>

Then, configure the plugin:

    <build>
        <plugins>
            <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>findbugs-maven-plugin</artifactId>
                <version>${findbugs.version}</version>
                <configuration>
                    <skip>${findbugs.skip}</skip>
                    <threshold>${findbugs.threshold}</threshold>
                    <findbugsXmlWithMessages>true</findbugsXmlWithMessages>
                    <excludeFilterFile>
                        ${findbugs.common},${findbugs.exclude}
                    </excludeFilterFile>
                    <jvmArgs>-Dfindbugs.glassfish.logging.validLoggerPrefixes=${findbugs.glassfish.logging.validLoggerPrefixes>}</jvmArgs>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.glassfish.findbugs</groupId>
                        <artifactId>findbugs</artifactId>
                        <version>1.5</version>
                    </dependency>
                </dependencies>
            </plugin>
        </plugins>
    </build>

And finally the site configuration:

    <!-- following to enable use of "mvn site:stage" -->
    <distributionManagement>
        <site>
            <id>oracle.com</id>
            <url>file:/tmp</url> <!-- not used -->
        </site>
    </distributionManagement>

    <reporting>
        <plugins>
            <plugin>
                <groupId>org.codehaus.mojo</groupId>
                <artifactId>findbugs-maven-plugin</artifactId>
                <version>${findbugs.version}</version>
                <configuration>
                    <skip>${findbugs.skip}</skip>
                    <threshold>${findbugs.threshold}</threshold>
                    <excludeFilterFile>
                        ${findbugs.common},${findbugs.exclude}
                    </excludeFilterFile>
                    <jvmArgs>-Dfindbugs.glassfish.logging.validLoggerPrefixes=${findbugs.glassfish.logging.validLoggerPrefixes>}</jvmArgs>
                </configuration>
                <!--
                <dependencies>
                    <dependency>
                        <groupId>org.glassfish.findbugs</groupId>
                        <artifactId>findbugs</artifactId>
                        <version>1.5</version>
                    </dependency>
                </dependencies>
                -->
            </plugin>
        </plugins>
    </reporting

Note that you'll need to update the "1.5" in the dependencies above to the version of the master GlassFish FindBugs exclude list that you want to depend on. The current version is 1.5.

In each module (if you have more than one) there might be an "exclude.xml" file (approved by me) that excludes certain FindBugs errors. The modules configure use of that file by setting a property in their pom.xml:

        <findbugs.exclude>${project.basedir}/exclude.xml</findbugs.exclude>

If some modules don't contain code that goes into the product, you can set <findbugs.skip>true</findbugs.skip> in those modules.

Post-4.0 Goals

Post-4.0 we'll be "raising the bar" for FindBugs errors.

In addition to the requirement of zero high priority errors (for all projects) and the requirement of no increase in the number of errors in each module (for the GlassFish project), we'd also like to reduce the number of medium and low priority errors (for the GlassFish project). The goal is:

  1. Reduce the number of bugs in any module to 20 or fewer.
  2. Reduce the total number of bugs in the following categories to zero.

Not all bugs have the same impact. We'd like to pay particular attention to bugs in these categories: CORRECTNESS, MALICIOUS_CODE, MT_CORRECTNESS, PERFORMANCE, SECURITY and reduce the number of such bugs to zero.

Remember that there are several ways to "fix" a FindBugs error:

  • Fix the code so that the error doesn't occur.
  • If the error report isn't really a bug, change the code so that FindBugs doesn't complain about it.
  • Convince me that the error report should be excluded (see below).

Process

The process will be similar to what we've done before:

  • You'll get immediate email if you violate any of the requirements described above. This is the same as breaking the build. You need to fix such problems immediately.
  • You'll get periodic email with your FindBugs errors. These are to help you in meeting the goals described above.
  • If you have an error report that you believe is wrong, convince me and I'll exclude it. It's easier to convince me about lower priority bugs.
  • If you believe an error type is almost always wrong, or almost never worth fixing (as above), convince me and I'll exclude it.
  • If it will take you more time to tell me why it's not an error than it does to "fix" it, fix it.

Attachments:
Glassfish_FindBugs.pdf (application/pdf)

Comments:

How is <excludeFilterFile>${findbugs.common},${findbugs.exclude}</excludeFilterFile>supposed to work? <excludeFilterFile> can only contain a single file name!?
Posted by 31603 at Jan 15, 2015 15:04