15

Update: Oracle has confirmed this as a bug.

Summary: Certain custom BeanInfos and PropertyDescriptors that work in JDK 1.6 fail in JDK 1.7, and some only fail after Garbage Collection has run and cleared certain SoftReferences.

Edit: This will also break the ExtendedBeanInfo in Spring 3.1 as noted at the bottom of the post.

Edit: If you invoke sections 7.1 or 8.3 of the JavaBeans spec, explain exactly where those parts of the spec require anything. The language is not imperative or normative in those sections. The language in those sections is that of examples, which are at best ambiguous as a specification. Furthermore, the BeanInfo API specifically allows one to change the default behavior, and it is clearly broken in the second example below.

The Java Beans specification looks for default setter methods with a void return type, but it allows customization of the getter and setter methods through a java.beans.PropertyDescriptor. The simplest way to use it has been to specify the names of the getter and setter.

new PropertyDescriptor("foo", MyClass.class, "getFoo", "setFoo");

This has worked in JDK 1.5 and JDK 1.6 to specify the setter name even when its return type is not void as in the test case below:

import java.beans.IntrospectionException;
import java.beans.PropertyDescriptor;
import org.testng.annotations.*;

/**
 * Shows what has worked up until JDK 1.7.
 */
public class PropertyDescriptorTest
{
    private int i;
    public int getI() { return i; }
    // A setter that my people call "fluent".
    public PropertyDescriptorTest setI(final int i) { this.i = i; return this; }

    @Test
    public void fluentBeans() throws IntrospectionException
    {
        // This throws an exception only in JDK 1.7.
        final PropertyDescriptor pd = new PropertyDescriptor("i",
                           PropertyDescriptorTest.class, "getI", "setI");

        assert pd.getReadMethod() != null;
        assert pd.getWriteMethod() != null;
    }
}

The example of custom BeanInfos, which allow the programmatic control of PropertyDescriptors in the Java Beans specification all use void return types for their setters, but nothing in the specification indicates that those examples are normative, and now the behavior of this low-level utility has changed in the new Java classes, which happens to have broken some code on which I am working.

There are numerous changes in in the java.beans package between JDK 1.6 and 1.7, but the one that causes this test to fail appears to be in this diff:

@@ -240,11 +289,16 @@
        }

        if (writeMethodName == null) {
-       writeMethodName = "set" + getBaseName();
+                writeMethodName = Introspector.SET_PREFIX + getBaseName();
        }

-       writeMethod = Introspector.findMethod(cls, writeMethodName, 1, 
-                 (type == null) ? null : new Class[] { type });
+            Class[] args = (type == null) ? null : new Class[] { type };
+            writeMethod = Introspector.findMethod(cls, writeMethodName, 1, args);
+            if (writeMethod != null) {
+                if (!writeMethod.getReturnType().equals(void.class)) {
+                    writeMethod = null;
+                }
+            }
        try {
        setWriteMethod(writeMethod);
        } catch (IntrospectionException ex) {

Instead of simply accepting the method with the correct name and parameters, the PropertyDescriptor is now also checking the return type to see whether it is null, so the fluent setter no longer gets used. The PropertyDescriptor throws an IntrospectionException in this case: "Method not found: setI".

However, the problem is much more insidious than the simple test above. Another way to specify the getter and setter methods in the PropertyDescriptor for a custom BeanInfo is to use the actual Method objects:

@Test
public void fluentBeansByMethod()
    throws IntrospectionException, NoSuchMethodException
{
    final Method readMethod = PropertyDescriptorTest.class.getMethod("getI");
    final Method writeMethod = PropertyDescriptorTest.class.getMethod("setI",
                                                                 Integer.TYPE);

    final PropertyDescriptor pd = new PropertyDescriptor("i", readMethod,
                                                         writeMethod);

    assert pd.getReadMethod() != null;
    assert pd.getWriteMethod() != null;
}

Now the above code will pass a unit test in both 1.6 and in 1.7, but the code will begin to fail at some point in time during the life of the JVM instance owing to the very same change that causes the first example to fail immediately. In the second example the only indication that anything has gone wrong comes when trying to use the custom PropertyDescriptor. The setter is null, and most utility code takes that to mean that the property is read-only.

The code in the diff is inside PropertyDescriptor.getWriteMethod(). It executes when the SoftReference holding the actual setter Method is empty. This code is invoked by the PropertyDescriptor constructor in the first example that takes the accessor method names above because initially there is no Method saved in the SoftReferences holding the actual getter and setter.

In the second example the read method and write method are stored in SoftReference objects in the PropertyDescriptor by the constructor, and at first these will contain references to the readMethod and writeMethod getter and setter Methods given to the constructor. If at some point those Soft references are cleared as the Garbage Collector is allowed to do (and it will do), then the getWriteMethod() code will see that the SoftReference gives back null, and it will try to discover the setter. This time, using the same code path inside PropertyDescriptor that causes the first example to fail in JDK 1.7, it will set the write Method to null because the return type is not void. (The return type is not part of a Java method signature.)

Having the behavior change like this over time when using a custom BeanInfo can be extremely confusing. Trying to duplicate the conditions that cause the Garbage Collector to clear those particular SoftReferences is also tedious (though maybe some instrumenting mocking may help.)

The Spring ExtendedBeanInfo class has tests similar to those above. Here is an actual Spring 3.1.1 unit test from ExtendedBeanInfoTest that will pass in unit test mode, but the code being tested will fail in the post-GC insidious mode::

@Test
public void nonStandardWriteMethodOnly() throws IntrospectionException {
    @SuppressWarnings("unused") class C {
        public C setFoo(String foo) { return this; }
    }

    BeanInfo bi = Introspector.getBeanInfo(C.class);
    ExtendedBeanInfo ebi = new ExtendedBeanInfo(bi);

    assertThat(hasReadMethodForProperty(bi, "foo"), is(false));
    assertThat(hasWriteMethodForProperty(bi, "foo"), is(false));

    assertThat(hasReadMethodForProperty(ebi, "foo"), is(false));
    assertThat(hasWriteMethodForProperty(ebi, "foo"), is(true));
}

One suggestion is that we can keep the current code working with the non-void setters by preventing the setter methods from being only softly reachable. That seems like it would work, but that is rather a hack around the changed behavior in JDK 1.7.

Q: Is there some definitive specification stating that non-void setters should be anathema? I've found nothing, and I currently consider this a bug in the JDK 1.7 libraries. Am I wrong, and why?

jbindel
  • 5,535
  • 2
  • 25
  • 38
  • 1
    I imagine this took quite a while to figure out. – biziclop May 29 '12 at 21:27
  • 1
    Heap dumps and VisualVM are my friends. – jbindel May 30 '12 at 00:22
  • Can you clarify or elaborate on the situation in which the posted code diff executes? – Pointy May 30 '12 at 00:24
  • I hope I clarified. It's in `getWriteMethod()`, which is called by the first example's constructor. In that case, and after the GC clears the Soft reference, it sees a null setter and tries to resolve that, but fails in the 1.7 code. – jbindel May 30 '12 at 00:42

4 Answers4

3

Looks like the specification hasn't changed (it requires void setter) but the implementation has been updated to only allow void setters.

Specification:

http://www.oracle.com/technetwork/java/javase/documentation/spec-136004.html

More specifically see section 7.1 (accessor methods) and 8.3 (design patterns for simple properties)

See some of the later answers in this stackoverflow question:

Does Java bean's setter permit return this?

Community
  • 1
  • 1
Mattias Isegran Bergander
  • 11,811
  • 2
  • 41
  • 49
  • Section 7.1 and 8.3 give examples of the "simple" and default patterns for accessors, but they do not *require* anything as far as I can determine. – jbindel May 29 '12 at 22:37
  • 2
    Being able to have bean APIs that deviate from the default seems like the basic point of the whole BeanInfo mechanism, and it's been that way since the '90s. – Pointy May 29 '12 at 22:38
1

Section 8.2 specifies:

However, within Java Beans the use of method and type names that match design patterns is entirely optional. If a programmer is prepared to explicitly specify their properties, methods, and events using the BeanInfo interface then they can call their methods and types whatever they like. However, these methods and types will still have to match the required type signatures, as this is essential to their operation.

(emphasis added)

Also, I beleive the method signatures shown in 7.1 and 8.3 are, in fact, normative. They are examples only in the sense that they use "foo" as an example property name.

James Scriven
  • 7,784
  • 1
  • 32
  • 36
  • The return type is not part of the "method signature", but maybe there is some definition of the "type signature" that is different. http://docs.oracle.com/javase/specs/jls/se5.0/html/classes.html#8.4.2 – jbindel May 30 '12 at 00:10
  • Re: 7.1 and 8.3 being normative, that's not how they are written. There's no "required", "must", "will", or "shall" in those, so it's hard for me to see how they are required. And Oracle still broke the behavior of PropertyDescriptors in 1.7. – jbindel May 30 '12 at 00:12
  • I *think* that the posted diff between 1.6 and 1.7 codes is executed only in cases in which a PropertyDescriptor has been lost to the GC. Thus, the enforcement of the `null` return type is only checked *after* the PropertyDescriptor may have been in use for an arbitrarily long period of time! – Pointy May 30 '12 at 00:26
  • I can look again, but I believe the constructor that takes the `String` getter and setter names calls `getWriteMethod()` as well, which is what the diff points to. – jbindel May 30 '12 at 00:39
  • Nothing about the return type is "essential to [the] operation" of a setter method, so I would not see that as a justification, either. – jbindel May 30 '12 at 02:57
  • @Fly, maybe there is a subtle difference between _**type** signature_ and _**method** signature_ – Mark Rotteveel May 31 '12 at 18:49
  • The former does not seem to be formally defined in the specifications. I'm sure the authors of the spec were smart enough to know that the void return type is never essential to the setter though, so that would not be what they are implying, i.e., to claim that is essential that way is a *non sequitur*. – jbindel May 31 '12 at 18:51
  • @MarkRotteveel That may be the way it was interpreted recently by someone, but I suspect it might simply have been an honest mistake that was not intended to change existing behavior. I'll post links to the bugs I've sent to Oracle once they're visible in the external database. – jbindel May 31 '12 at 19:23
  • @Fly I went over the spec and I think you are right that it is a misinterpretation of the spec; although the spec is really unspecific and unclear at this. – Mark Rotteveel Jun 02 '12 at 08:45
  • Thanks. I agree that the spec is really more of a white paper than a formal spec. It's easy to read, but not always as clearly authoritative as it should be. I've submitted bugs to Oracle, and I'm waiting to see how they are handled. The bugs are not yet visible in the external Java bug database. – jbindel Jun 02 '12 at 14:43
1

I would also opt to say disallowing non-void setters is an error. It simply makes fluent programming impossible. Thats why it needs to be be changed.

Thomas Nagel
  • 106
  • 3
0

Since I found Spring 3.1.1 ExtendedBeanInfo unit tests that expect the code not to be broken, and because having the behavior change after garbage collection is obviously a bug, I shall answer this and note the Java bug numbers. The bugs are still not visible on the external database of Java bugs, but I hope that they will become visible at some point:

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7172854 (Oracle closed this as a duplicate of the bug below since they have the same cause despite different manifestations.)

http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7172865

(The bugs were submitted on May 30, 2012.)

As of June 20, 2012, the bugs are visible in the external database via the links above.

jbindel
  • 5,535
  • 2
  • 25
  • 38
  • Second link redirects to http://bugs.java.com/bugdatabase/view_bug.do?bug_id=7172865 and is not accessible. I can only see title of the second since first links to it: "JDK-7172865 - PropertyDescriptor fails to work with setter method name if setter is non-void". Sad, Oracle :( – Piotr Findeisen Mar 25 '14 at 08:13
  • I noticed that, too, so I don't know the status of the bugs. One can check the source code to see whether it's been fixed. – jbindel Mar 25 '14 at 13:28
  • 1
    It seems sufficient to press 'search again' button and type the bug id into proper field to see those bug reports. Nothing optimistic to look at, anyway. Especially since the "getWriteMethod() is broken after GC clears SoftReferences" is true for use cases with generics, no need to call `setWriteMethod` to have a big problem with this. – Piotr Findeisen Apr 14 '14 at 10:03