Find java application bugs with findbugs

Java Bugs with Static Analysis Tool findbugs Help Improve Software Code Quality

This post is regarding the static analysis tool that finds real bugs in Java programs that help to improve software code quality. Static analysis tools can find real defects and issues in the code. We can effectively incorporate static analysis into our software development process.

FindBugs - Static analysis Tool

FindBugs is an open source static analysis tool that analyzes Java class files, looking for programming defects. The analysis engine reports nearly 300 different bug patterns. Each bug pattern is grouped into a category example,

• correctness
• bad practice
• performance
• internationalization

and each report of a bug pattern is assigned a

• priority
• high
• medium
• low

Let’s start with some of the selected bug categories with some examples.

Correctness

Comparing incompatable types for equality

Consider the following code,

if ((!value.equals(null)) && (!value.equals(""))) {
    Map spaces = (Map) vm.get(SpaceConstants.AVAILABLESPACEMAP);
}

One would expect that the condition would true, when value is not null and is not empty. However, value.equals(null) according to the contract of the equals() method, would always return false.

Consider the another similar example,

if ((bean.getNoteRate() != null) && 
    !bean.getNoteRate().equals("") && 
    (bean.getNoteRate() > 0)) {
          item.setNoteRate(bean.getNoteRate());
}

We might expect that the condition would true, when noteRate is not null, not empty and is greater than 0. However, the condition would never be true.

The reason is that bean.getNoteRate().equals("") would always return false regardless of being equal value.

According to the contract of equals(), objects of different classes should always compare as unequal; therefore, according to the contract defined by java.lang.Object.equals(Object), the result of this comparison will always be false at runtime.

Null pointer dereference

Consider the following code,

if ((list == null) && (list.size() == 0)) {
     return null;
}

This will lead to a Null Pointer Exception when the code is executed while list is null.

Suspicious reference comparison

Consider the following code,

if (bean.getPaymentAmount() != null && 
    bean.getPaymentAmount() != currBean.getPrincipalPaid()) { 
        // code to execute
}

This code compares two reference values (Double paymentAmount) using the != operator, where the correct way to compare instances of this type is generally with the equals() method. It is possible to create distinct instances that are equal but do not compare as == since they are different objects.

Doomed test for equality to NaN

Consider the following code,

if ((newValue == Double.NaN) || (newValue < 0d)) {
    // the code to execute
}

This code checks to see if a floating point value is equal to the special Not A Number value. However, because of the special semantics of NaN, no value is equal to Nan, including NaN. Thus, x == Double.NaN always evaluates to false. To check if a value contained in 'x' is the special Not A Number value, use Double.isNaN(x) (or Float.isNaN(x) if x is floating point precision).

Also see How can you compare NaN values? .

Method whose return value should not ignore

string is immutable object. So ignoring the return value of the method would consider as bug.

String name = "Muhammad";
name.toUpper();
if (name.equals("MUHAMMAD"))

Performance

Method invokes inefficient Boolean constructor;use Boolean.valueOf(...) instead

Consider the following code,

if ((record.getAmount() != null) && 
    !record.getAmount().equals(new Boolean(bean.isCapitalizing()))) { 
           // code to execute
}

Creating new instances of java.lang.Boolean wastes memory, since Boolean objects are immutable and there are only two useful values of this type. Use the Boolean.valueOf() method (or Java 1.5 autoboxing) to create Boolean objects instead.

Inefficient use of keySet iterator instead of entrySet iterator

Consider the following code,

Iterator iter = balances.keySet().iterator();     
while (iter.hasNext()) { 
    // code to execute
}

This method accesses the value of a Map entry, using a key that was retrieved from a keySet iterator. It is more efficient to use an iterator on the entrySet of the map, to avoid the Map.get(key) lookup.

Method invokes inefficient Number constructor; use static valueOf instead

Consider the following code,

Integer number1 = new Integer(123);
Integer number2 = Integer.valueOf(123);
System.out.println("number1 =  " + number1);
System.out.println("number2 =  " + number2);

Using new Integer(int) is guaranteed to always result in a new object whereas Integer.valueOf(int) allows caching of values to be done by the class library, or JVM. Using of cached values avoids object allocation and the code will be faster.

Also see: Integer Auto Boxing

Method concatenates strings using + in a loop

for (int x = 0; x < exceptions.size(); x++) {
    errorMessage += getStackTrace(
         exceptions.get(x) + "\n");
}

In each iteration, the String is converted to a StringBuffer/StringBuilder, appended to, and converted back to a String. This can lead to a cost quadratic in the number of iterations, as the growing string is recopied in each iteration.

Better performance can be obtained by using a StringBuilderexplicitly.

Dodgy

Code that is confusing, anomalous, or written in a way that leads itself to errors. Examples include dead local stores, switch fall through, unconfirmed casts, and redundant null check of value known to be null.

instanceof will always return true

The instanceof test will always return true (unless the value being tested is null)

NodeList nodeList = root.getElementsByTagName("node");
int nodeListLength = nodeList.getLength();
for (int i = 0; i < nodeListLength; i++) {
   Node node = nodeList.item(i);
   if (node instanceof Node &&  
       node.getParentNode() == root) {
           //do code
   }
}

Test for floating point equality

Consider the following code,

private double value = 0d;
if (value > diff) {
     // code to excute
} else if (value == diff) {     
    // code to excute
}

The above code compares two floating point values for equality. Because floating point calculations may involve rounding, calculated float and double values may not be accurate. For values that must be precise, such as monetary values, BigDecimal would be more appropriate.

See Floating-Point Operations

Also see Effective Java 2nd Ed, Item 48:Avoid float and double if exact answers are required

Integral division result cast to double or float

Consider the code,

int x = 2;
int y = 5;
// Wrong: yields result 0.0
double value1 =  x / y;

This code casts the result of an integral division operation to double or float. Doing division on integers truncates the result to the integer value closest to zero. The fact that the result was cast to double suggests that this precision should have been retained. We should cast one or both of the operands to double before performing the division like

// Right: yields result 0.4
double value2 =  x / (double) y;

References:

• Download the latest version of FindBugs
• Visit the blog
• Using the FindBugs Ant task
• Using the FindBugs Eclipse plugin
• Using the PMD
• Visit the blog

Phantom References in Java

Memory management is done automatically in Java. The programmer doesn't need to worry about reference objects that have been released. One downside to this approach is that the programmer cannot know when a particular object will be collected. Moreover, the programmer has no control over memory management. However, the java.lang.ref package defines classes that provide a limited degree of interaction with the garbage collector. The concrete classes SoftReference, WeakReference and PhantomReference are subclasses of Reference that interact with the garbage collector in different ways. In this article we will discuss the functionality and behavior of the PhantomReference classes and see how it can be used.

Problem with Finalization

To perform some postmortem cleanup on objects that garbage collector consider as unreachable, one can use finalization. This feature can be utilized to reclaim native resources associated with an object. However, finalizers have many problems associated.

Firstly, we can’t foresee the call of finalize(). Since the Garbage Collection is unpredictable, the calling of finalize() cannot be predicted. There is no guarantee that the object will be garbage collected. The object might never become eligible for GC because it could be reachable through the entire lifetime of the JVM. It is also possible that no garbage collection actually runs from the time the object became eligible and before JVM stops.

Secondly, Finalization can slowdown an application. Managing objects with a finalize() method takes more resources from the JVM than normal objects.

As per doc,

You should also use finalization only when it is absolutely necessary. Finalization is a nondeterministic -- and sometimes unpredictable -- process. The less you rely on it, the smaller the impact it will have on the JVM and your application

In Effective Java, 2nd ed., Joshua Bloch says,

there is a severe performance penalty for using finalizers... So what should you do instead of writing a finalizer for a class whose objects encapsulate resources that require termination, such as files or threads? Just provide an explicit termination method, and require clients of the class to invoke this method on each instance when it is no longer needed.

In short, Finalize() isn't used often, and also there is no much reason to use it. If we have a class with methods like close() or cleanup() and that should be called once user done with the object then placing these methods call in finalize() can be used as a safety measure, but not necessary.

Phantom Reference
phantom reachable, phantomly reachable

An object is phantom reachable if it is neither strongly nor softly nor weakly reachable and has been finalized and there is a path from the roots to it that contains at least one phantom reference.

The PhantomReference constructor accepts two arguments:

referent - the object the new phantom reference will refer to
q - the reference is registered with the given queue.

The argument q represents the instance of the ReferenceQueue class. If the garbage collector determines that the referent of a phantom reference is phantom reachable, then the PhantomReference will be added to this ReferenceQueue. You can then retrieve the PhantomReference by using the remove() methods of the ReferenceQueue class.

Consider the following example,

ReferenceQueue q = new ReferenceQueue();
PhantomReference pr = new PhantomReference(object, referenceQueue);

// Later on another point
Reference r = q.remove();

// Now, clear up any thing you want

PhantomReference, when to use?
Phantom Reference can be used in situations, where sometime using finalize() is not sensible thing to do.This reference type differs from the other types defined in java.lang.ref Package because it isn't meant to be used to access the object, but as a signal that the object has already been finalized, and the garbage collector is ready to reclaim its memory.

As per API doc,

Phantom reference objects, which are enqueued after the collector determines that their referents may otherwise be reclaimed. Phantom references are most often used for scheduling pre-mortem cleanup actions in a more flexible way than is possible with the Java finalization mechanism.

People usually attempt to use finalize() method to perform postmortem cleanup on objects which usually not advisable. As mentioned earlier, Finalizers have an impact on the performance of the garbage collector since Objects with finalizers are slow to garbage collect.

Phantom references are safe way to know an object has been removed from memory. For instance, consider an application that deals with large images. Suppose that we want to load a big image in to memory when large image is already in memory which is ready for garbage collected. In such case, we want to wait until the old image is collected before loading a new one. Here, the phantom reference is flexible and safely option to choose. The reference of the old image will be enqueued in the ReferenceQueue once the old image object is finalized. After receiving that reference, we can load the new image in to memory.

Similarly we can use Phantom References to implement a Connection Pool. We can easily gain control over the number of open connections, and can block until one becomes available.

Reference Objects and Garbage Collection

Soft Reference can be garbage collected after there are no strong references to the referent. However, it typically retained until memory is low. All softly reachable objects will be reclaimed before an OutOfMemoryException is thrown. Therefore, it can be used to implement caches of objects that can be recreated if needed.

Weak Reference can be garbage collected when there are no strong or soft references to the referent. However, unlike Soft Reference, they are garbage collected on a gc even when memory is abundant. They often can be used for “canonical mappings” where each object has a unique identifier (one-to-one), and in collections of “listeners”

On the other hand, Phantom Reference, can be garbage collected once there are no strong, soft or weak references to the referent. When object is phantomly reachable, it means the object is already finalized but not yet reclaimed, so the GC enqueues it in a ReferenceQueue for post-finalization processing.

As per Java Doc,

Unlike soft and weak references, phantom references are not automatically cleared by the garbage collector as they are enqueued. An object that is reachable via phantom references will remain so until all such references are cleared or themselves become unreachable.

A PhantomReference is not automatically cleared when it is enqueued, so when we remove a PhantomReference from a ReferenceQueue, we must call its clear() method or allow the PhantomReference object itself to be garbage-collected.

Summary

In short, we should avoid finalize() as much as possible. There is no guarantee if the finalize() method will be called promptly following garbage collection, or even it will be called. If the finalize method runs for a long time, it can delay execution of finalize methods of other objects. Instead of relying on finalize(), we can use reference types define in java.lang.ref package.

Beside java.lang.ref package, Google collection library also provide some alternatives. For example, FinalizablePhantomReference extends java.lang.ref.PhantomReference, deals with processing the ReferenceQueue and call back a convenient method finalizeReferent(). So if we want to do some cleanup operation when an object is claimed by the garbage collector (GC) then we just need to override the finalizeReferent() method.

Resources

Garbage Collection
PhantomReference
The java.lang.ref Package (Java in a Nutshell)
Understanding Weak References