Hi folks,
ICU4J implements Java locale service provider interface. I recently
received a problem report from our customer. When they upgraded Java
version to 8, the provider implementation stopped working because of NPE
thrown by our custom DateFormatSymbols subclass. I dug into the problem
and found that DateFormatSymbols caches its own instance in the
constructor. Our subclass implementation expects a field is always
initialized to non-null in the constructor. However, clone() method is
called for the super class's constructor, the field is not yet
initialized at the point.
It looks adding 'null' for the field in clone() method would resolve the
immediate problem, but I'm not comfortable that DateFormatSymbols
implementation cache a premature instance of my own subclass. If any
methods overridden by my own subclass is called on a premature instance,
it might cause another issue.
Anyway, I filed a bug in the Java bug database with the description
below. For meanwhile, adding simple 'null' check in our code would
suffice. But I would like to hear Java i18n team's opinion about this issue.
Thanks,
Yoshito
---------------------------
To implement my own locale service provider, I have a class extending
java.text.DateFormatSymbols. My custom subclass's constructor implicitly
invokes the no-args constructor in java.text.DateFormatSymbols. The
constructor calls a private method - initializeData(Locale).
It looks the implementation was updated in Java 8 and initializeData is
now trying to cache an instance of DateFormatSymbols at the end and
calls this.clone().
In my own subclass implements clone() method, which copies a field
initialized by a constructor in the class. For example,
===============
public class MyDateFormatSymbols extends DateFormatSymbols {
private final Foo foo;
public MyDateFormatSymbols(Foo foo) {
if (foo == null) {
this.foo = new Foo();
} else {
this.foo = foo;
}
}
@Override
public Object clone() {
MyDateFormatSymbols mdfs = (MyDateFormatSymbols)super.clone();
mdfs.foo = this.foo.clone();
}
}
===============
When the constructor MyDateFormatSymbols(Foo) is called, it triggers
no-args constructor of the super class - DateFormatSymbols() first. As I
explained earlier, Java 8 implementation calls this.clone() in
DateFormatSymbpls.initializeData(Locale). At that point, the field foo
in my class is not yet initialized, so this.foo.clone() will throw
NullPointerException.
My own code expects the field 'foo' is always non-null. I could change
clone() to check if this.foo is null or not, but I cannot control cached
'premature' instance held by Java DateFormatSymbols. At this moment, it
looks the cache is only used for copying field values maintained by
DateFormaSymbols itself and never call a method. So, even 'premature'
instance of my own subclass instance is referenced by DateFormatSymbols,
it won't cause any problems. However, if the Java's implementation is
changed to call any DateFormatSymbols method overridden by my own
subclass, it may not work (because my subclass expects the field foo is
non-null).
Such code above did not have any problems with earlier Java releases
(Java 6 / 7).
In my opinion, this.clone() should not be called in DateFormatSymbols
initialization code. Instead, it should create a private container class
for these symbols, and cache the object, not DateFormatSymbols itself.
