Pages

dimanche 11 septembre 2011

Java Puzzler - Defensive Serialization

It has been a long time since I didn't write on Pingtimeout. Sorry for that :)

Yesterday, while I was adding some features to JTailPlus, I ran into an XML Serialization problem. I had some difficulties to find out what was wrong, and I must admit I didn't expect this kind of error to occur. In this article, I will expose the problem I ran into, and the solution.

First of all, if you don't have read the book "Java Puzzlers" yet, I urge you to do so. It is a must-read for every java developer. It is all about "Traps, Pitfalls, and Corner Cases" of the Java programming language. I enjoyed myself reading the puzzles, even if sometimes I almost tore my hair out.

As this book does, I will present the problem I ran into as a Puzzler. You will see the code and will be asked to guess what it produce. Let's begin.

Puzzler

The following class represents a person by it's first name, last name and children. The library commons-io is used to close streams. In the main method, we serialize and deserialize a person (aka John Jackson). What does the program print ?

What do we expect ?

This program does not seem weird. The class Person is a java bean and it is serializable. The main method creates three persons (John Jackson and his two children, Jim and Barbara), John is then serialized to XML with the XMLEncode class and deserialized using the XMLDecoder class. Therefore the program should print :

John Jackson's children are [Jim Jackson, Barbara Jackson]
John Jackson's children are [Jim Jackson, Barbara Jackson]

What do we have ?

If you ran it, you saw a different thing :

John Jackson's children are [Jim Jackson, Barbara Jackson]
java.lang.NullPointerException
Continuing ...
John Jackson

We lost two kids in the process, what can be wrong ?

Solution

The only unusual thing in our program is the use of the class Collections to populate the children attribute and to return an unmodifiable version of the list when the getter is called. We assume that the author of this code did not want anyone but the class Person itself to modify the list "children". Except that, we have a standard Java Bean.

The addAll method seems pretty straightforward : "The behavior of this convenience method is identical to that of c.addAll(Arrays.asList(elements)), but this method is likely to run significantly faster under most implementations.", Javadoc says. Sweet, a fast implementation.

The unmodifiableList method also seem simple : it returns "an unmodifiable view of the specified list. [...] attempts to modify the returned list, whether direct or via its iterator, result in an UnsupportedOperationException. [...] The returned list will be serializable if the specified list is serializable.".

Our initial is is an ArrayList, which is serializable, so the generated list is serializable as well. The console output does not show an UnsupportedOperationException but a NullPointerException. This does not help us very much.

Actually, the bug _is_ caused by the defensive copying design. If we replace the getter by another defensive copying method, the error is still there, John Jackson's kids are still lost :

The answer lies in the definition of a Java Bean itself. According to wikipedia, the assumed conventions for Java Beans are :

The class must have a public default constructor (no-argument).

The class properties must be accessible using get, set, is (used for boolean
properties instead of get), [...] following a standard naming-convention.

The class should be serializable.

The author of this code (me) did not follow the second convention : the "children" attribute is not directly accessible using its appropriate getter, instead, another object is returned. If we replace the getter statement by the following, everything works fine and John Jackson's children are back.

public List<Person> getChildren() { return children; }

The lesson of this puzzle is : be careful when dealing with java beans and defensive copying. The defensive copying design, which can be a valid choice in numerous situations, is inappropriate here.