The Windward Studio

Windward Blog Home

Why Java Programmers Should Never Use Object == Object

by
Posted on 03/26/2014

Please Share This

 

We used to use Object == Object in our Java Engine code. But recently one of our customers hit a problem, and finding the error was very difficult. We changed the Engine code so this won’t happen again. More importantly, knowing why it happened might save you boatloads of time in your own Java applications.

It Looks Simple but It’s Not

We all understand the difference between == (same object) and Object.Equals (objects may be different, but the content is the same). But it’s not quite that simple in the case of ==. Let’s look at how we think it works:

int i1 = 1000;
int i2 = 1000;
IntegerPlus o1 = new IntegerPlus  (1000);
IntegerPlus o2 = new IntegerPlus (1000);
IntegerPlus o3 = o1;

boolean b1 = i1 == i2;
boolean b2 = o1 == o2;
boolean b3 = o1.Equals(o2);

boolean b4 = o1 == o3;
boolean b5 = o1.Equals (o3);

In the above code b1 & b3 will always be true. And b2 will be false because o1 is a different object from o2.

But now let’s look at the remaining two cases. b4 & b5 will usually be true, but can be false. Yes, you are comparing two references to the same object and the equality test can fail. What I think is happening (the companies creating JVMs say virtually nothing about how the JVMs are implemented) is as follows:

o1 & o3 are references to an object. That object can be moved in memory when the garbage collector runs. The == uses System.identityHashCode() to determine if the objects are identical. I believe this is the address of the object in memory at the time the object is created. So o1 is created & identiyHashCode is assigned to it, the object is moved, then it is assigned to a new reference o3 which then makes its own call to identityHashCode and has a different number.

If you’re placing an object in a collection, I believe it can be even more volatile as the JVM stores objects differently (using less memory per object) in a collection to reduce memory usage. And collections are an obvious choice for relocation by the garbage collector as they tend to use a lot of memory. So placement in an array can make things even more worrisome.

What about Object.Equals()? If you write your own, no problem. But if don’t, and the call goes down to Object.Equals(), then once again the JVM is using System.identityHashCode() and you might face the same problem. I don’t know for sure because Equals() might call System.identityHashCode() when it executes and in that case you’re very safe (although not 100% as the GC could run between the 2 calls to it). Bottom line, write your own Equals and don’t call super.Equals if you inherit directly from Object.

Yeah, It Happened to Us

Can this really occur? Yes! We missed one spot in the Engine when we addressed this about 5 years ago. Here’s the code in the Engine as of 2 weeks ago:

while(stack.size() > 0) {
TinyWriteElement elem = (TinyWriteElement) stack.get(stack.size()-1);
if (elem == element)
return;
stack.remove(stack.size()-1);
}
throw new IllegalArgumentException(“element ” + element.getName() + ” is already written”);

The above is in use on tens (hundreds?) of thousands of systems. It has run untold millions of reports for the last 10 years (it’s used for WordML, DOCX, PPTX, & XSLX output). And has done so with no issues.

But two weeks ago one of our customers hit an issue where it was throwing the exception under intense load. elem and element were references to the same object where the object was pushed on the stack, the code then writes out to the XML file the attributes and sub-elements of the object, then needs to pop it off the stack as that element is closed. During that processing time, when the garbage collector ran, it changed one (or both) the references to this same object so that an == would no longer evaluate as true.

We changed the code as follows:

while(stack.size() > 0) {
TinyWriteElement elem = (TinyWriteElement) stack.get(stack.size()-1);
if (elem.getReferenceId() == element.getReferenceId())
return;
stack.remove(stack.size()-1);
}
throw new IllegalArgumentException(“element ” + element.getName() + ” is already written”);

Where getReferenceId() returns an int. This works 100%. (We’ve had getReferenceId() in there for years for this very reason, we just missed this one case where we needed it). This is set up as follows:

private static int nextReferenceId = 1;
private int referenceId;
TinyWriteElement() {
referenceId = nextReferenceId++;
}
public int getReferenceId() {
return referenceId;
}

These objects exist only for a short period while generating the output report where an XML file is needed (WordML, DOCX, PPTX, & XLSX). So yes the int will roll over to 0 eventually, but when it does, the previous use of 0 will be long gone.

Best Practice

This is something you will rarely hit, but that makes finding the error when it does hit very, very difficult. (And as far as I know, this is not an issue with C#.) Best practice is:

  1. Do not use == for Java objects (on primitives it’s fine).
  2. Do not use/call Object.Equals(). Watch out for what super.Equals() is calling if you have it in your Equals.

For more useful tips and answers about the Java Engine, visit our newly updated Documentation Wiki.

Please Share This



Author: David Thielen

Dave, Windward's founder and CEO, is passionate about building superb software teams from scratch and dramatically improving the productivity of existing software teams. He's really proud that he once created a game so compelling (Enemy Nations) that a now-professional World of Warcraft player lost his job for playing it incessantly on company time. You can read more from Dave on his personal blog, and at Huffington Post.

Other posts by