Problem with JDK 1.7 and Twig

83 views
Skip to first unread message

Rick Horowitz

unread,
Oct 22, 2012, 2:57:48 PM10/22/12
to twig-p...@googlegroups.com
I'm trying to upgrade to JDK 1.7 due to what looks like a bug in the latest version of app engine plugin for Eclipse when trying to perform a URL fetch. A posting online indicated that the problem that I am experiencing with JDK 1.6 can be fixed by using the Oracle version of JDK 1.7, hence I have installed that, but now I experience the following problem with Twig. (Note: this problem is triggered from an ObjectDatastore.find() command):

Caused by: java.lang.IllegalArgumentException: Cannot compare with [SimpleProperty value=null path=phoneNbr indexed=false]
at com.google.code.twig.util.SimpleProperty.compareTo(SimpleProperty.java:110)
at com.google.code.twig.util.SimpleProperty.compareTo(SimpleProperty.java:6)
at java.util.TreeMap.compare(TreeMap.java:1188)
at java.util.TreeMap.put(TreeMap.java:531)
at java.util.TreeSet.add(TreeSet.java:255)
at java.util.AbstractCollection.addAll(AbstractCollection.java:334)
at java.util.TreeSet.addAll(TreeSet.java:312)
at com.google.code.twig.standard.StandardDecodeCommand.entityToInstance(StandardDecodeCommand.java:114)
at com.google.code.twig.standard.StandardDecodeCommand.keyToInstance(StandardDecodeCommand.java:205)
at com.google.code.twig.standard.StandardUntypedSingleLoadCommand.keyToInstance(StandardUntypedSingleLoadCommand.java:6)
at com.google.code.twig.standard.StandardUntypedSingleLoadCommand.now(StandardUntypedSingleLoadCommand.java:19)
at com.google.code.twig.standard.RelationTranslator.keyToInstance(RelationTranslator.java:131)
at com.google.code.twig.standard.ParentRelationTranslator.decode(ParentRelationTranslator.java:38)
at com.google.code.twig.translator.FieldTranslator.decodeField(FieldTranslator.java:158)

What is happening is that an entity property with a null value is being compared to itself. This line 99, path.compareTo(o.getPath()) to return 0. This causes the first else clause (line 104) to be skipped, leading to throw new IllegalArgumentException() at line 110.

Two observations:

1. line 106 - I think is in error (compares to itself. Should be? -- return compare((Comparable<?>) value, o.getValue());
2. It looks like some else clauses need to be added to test for value or o.value being null and returning the appropriate value from SimpleProperty.compareTo().

Any help with this would be appreciated. The code looks a bit inadequate, but I am surprised that this hasn't been seen before. Is no one using JDK 1.7?

Thanks,

Rick

John Patterson

unread,
Oct 22, 2012, 9:55:59 PM10/22/12
to twig-p...@googlegroups.com
Looks like that code is only used when the paths are the same between the two SimpleProperty's being compared. If the paths are different then the value comparison is never made.

How does this happen? When an Entity is loaded you should never get two properties with the same name.

If you can recreate the problem some how it will help me find out whats going on.

John Patterson

unread,
Oct 23, 2012, 1:14:17 AM10/23/12
to twig-p...@googlegroups.com
Also, I pushed a whole bunch of updates today that have accumulated in my local git repository.  Its possible that a fix has already been made?

Rick Horowitz

unread,
Oct 23, 2012, 8:21:49 AM10/23/12
to twig-p...@googlegroups.com
John, thanks for the quick response. I need to rebuild my Eclipse workspace. Seeing funny errors -- I think it's corrupted. Will be back to you when I'm setup again... Rick


On Tue, Oct 23, 2012 at 1:14 AM, John Patterson <jdpat...@gmail.com> wrote:
Also, I pushed a whole bunch of updates today that have accumulated in my local git repository.  Its possible that a fix has already been made?




--
Rick Horowitz

Rick Horowitz

unread,
Oct 23, 2012, 9:01:49 PM10/23/12
to twig-p...@googlegroups.com
John, I video-recorded a debugger session, basically single stepping from the starting point in Twig until the IllegalArgumentException is thrown. I'm running the latest version of Twig. Here's the video: http://screencast-o-matic.com/watch/cl6ZcrS3V. 

Stack Trace:

java.lang.IllegalArgumentException: Cannot compare with [SimpleProperty value=null path=personKey indexed=false]
at com.google.code.twig.util.SimpleProperty.compareTo(SimpleProperty.java:110)
at com.google.code.twig.util.SimpleProperty.compareTo(SimpleProperty.java:6)
at java.util.TreeMap.compare(TreeMap.java:1188)
at java.util.TreeMap.put(TreeMap.java:531)
at java.util.TreeSet.add(TreeSet.java:255)
at java.util.AbstractCollection.addAll(AbstractCollection.java:334)
at java.util.TreeSet.addAll(TreeSet.java:312)
at com.google.code.twig.standard.StandardDecodeCommand.entityToInstance(StandardDecodeCommand.java:112)
at com.google.code.twig.standard.StandardDecodeCommand$1.next(StandardDecodeCommand.java:164)
at com.google.common.collect.ForwardingIterator.next(ForwardingIterator.java:48)
at com.toolcafe.gwtstore.server.GSAdminServiceImpl.createBaseAdmin(GSAdminServiceImpl.java:46)

Thanks... Rick

John Patterson

unread,
Oct 23, 2012, 9:17:34 PM10/23/12
to twig-p...@googlegroups.com
Hey thats quite cool :)  Seems so familiar!

I still cannot see how two properties are being added that have the same path.  Properties should always have different paths.  This problem does look familiar though.  I am sure I have seen it some time before.

Next step: find out how two properties are added with same path.

Rick Horowitz

unread,
Oct 23, 2012, 10:52:23 PM10/23/12
to twig-p...@googlegroups.com
It looks like TreeMap.put() has been changed from OpenJDK 6 to Oracle JDK 7. Here's a diff of the relevant section, followed by the full put() method for each version.

531,533c530,535
<             compare(key, key); // type (and possibly null) check
< 
<             root = new Entry<>(key, value, null);
---
>             // TBD:
>             // 5045147: (coll) Adding null to an empty TreeSet should
>             // throw NullPointerException
>             //
>             // compare(key, key); // type check
>             root = new Entry<K,V>(key, value, null);

Note that the compare(key, key); that was commented out in OpenJDK 6 is no longer commented out. This method is now called, and results in the call to SimpleProperty.compareTo(). Take another look the suggestions from my first posting in this thread. I think they may help fix the problem.

1. line 106 - I think is in error (compares to itself. Should be? -- return compare((Comparable<?>) value, o.getValue());
2. It looks like some else clauses need to be added to test for value or o.value being null and returning the appropriate value from SimpleProperty.compareTo().

TreeMap.put() method from Oracle JDK 7: (starts at line 528)


    /**
     * Associates the specified value with the specified key in this map.
     * If the map previously contained a mapping for the key, the old
     * value is replaced.
     *
     * @param key key with which the specified value is to be associated
     * @param value value to be associated with the specified key
     *
     * @return the previous value associated with {@code key}, or
     *         {@code null} if there was no mapping for {@code key}.
     *         (A {@code null} return can also indicate that the map
     *         previously associated {@code null} with {@code key}.)
     * @throws ClassCastException if the specified key cannot be compared
     *         with the keys currently in the map
     * @throws NullPointerException if the specified key is null
     *         and this map uses natural ordering, or its comparator
     *         does not permit null keys
     */
    public V put(K key, V value) {
        Entry<K,V> t = root;
        if (t == null) {
            compare(key, key); // type (and possibly null) check

            root = new Entry<>(key, value, null);
            size = 1;
            modCount++;
            return null;
        }
        int cmp;
        Entry<K,V> parent;
        // split comparator and comparable paths
        Comparator<? super K> cpr = comparator;
        if (cpr != null) {
            do {
                parent = t;
                cmp = cpr.compare(key, t.key);
                if (cmp < 0)
                    t = t.left;
                else if (cmp > 0)
                    t = t.right;
                else
                    return t.setValue(value);
            } while (t != null);
        }
        else {
            if (key == null)
                throw new NullPointerException();
            Comparable<? super K> k = (Comparable<? super K>) key;
            do {
                parent = t;
                cmp = k.compareTo(t.key);
                if (cmp < 0)
                    t = t.left;
                else if (cmp > 0)
                    t = t.right;
                else
                    return t.setValue(value);
            } while (t != null);
        }
        Entry<K,V> e = new Entry<>(key, value, parent);
        if (cmp < 0)
            parent.left = e;
        else
            parent.right = e;
        fixAfterInsertion(e);
        size++;
        modCount++;
        return null;
    }

OpenJDK 6 TreeMap.put() method:


    /**
     * Associates the specified value with the specified key in this map.
     * If the map previously contained a mapping for the key, the old
     * value is replaced.
     *
     * @param key key with which the specified value is to be associated
     * @param value value to be associated with the specified key
     *
     * @return the previous value associated with <tt>key</tt>, or
     *         <tt>null</tt> if there was no mapping for <tt>key</tt>.
     *         (A <tt>null</tt> return can also indicate that the map
     *         previously associated <tt>null</tt> with <tt>key</tt>.)
     * @throws ClassCastException if the specified key cannot be compared
     *         with the keys currently in the map
     * @throws NullPointerException if the specified key is null
     *         and this map uses natural ordering, or its comparator
     *         does not permit null keys
     */
    public V put(K key, V value) {
        Entry<K,V> t = root;
        if (t == null) {
            // TBD:
            // 5045147: (coll) Adding null to an empty TreeSet should
            // throw NullPointerException
            //
            // compare(key, key); // type check
            root = new Entry<K,V>(key, value, null);
            size = 1;
            modCount++;
            return null;
        }
        int cmp;
        Entry<K,V> parent;
        // split comparator and comparable paths
        Comparator<? super K> cpr = comparator;
        if (cpr != null) {
            do {
                parent = t;
                cmp = cpr.compare(key, t.key);
                if (cmp < 0)
                    t = t.left;
                else if (cmp > 0)
                    t = t.right;
                else
                    return t.setValue(value);
            } while (t != null);
        }
        else {
            if (key == null)
                throw new NullPointerException();
            Comparable<? super K> k = (Comparable<? super K>) key;
            do {
                parent = t;
                cmp = k.compareTo(t.key);
                if (cmp < 0)
                    t = t.left;
                else if (cmp > 0)
                    t = t.right;
                else
                    return t.setValue(value);
            } while (t != null);
        }
        Entry<K,V> e = new Entry<K,V>(key, value, parent);
        if (cmp < 0)
            parent.left = e;
        else
            parent.right = e;
        fixAfterInsertion(e);
        size++;
        modCount++;
        return null;
    }

John Patterson

unread,
Oct 23, 2012, 11:07:51 PM10/23/12
to Rick Horowitz, twig-p...@googlegroups.com
Excellent investigation!

I'll get that fix in there asap. Very interesting to know that an item will now be compared to itself when adding to TreeMap.

Thanks for looking into this

Rick Horowitz

unread,
Oct 24, 2012, 10:17:24 AM10/24/12
to twig-p...@googlegroups.com, Rick Horowitz
The fix looks good! I'll let you know if I see any further problems, and fortunately, the upgrade to JDK 7 eliminated the Google bug that instigated my upgrade in the first place. 

BTW, according to my estimates, you provide bug fixes approximately 864,238 times faster than Google does. ;-)

Arnaud Mary

unread,
Oct 24, 2012, 5:00:49 PM10/24/12
to twig-p...@googlegroups.com, Rick Horowitz
Thanks for working on this, guys. I just met the same problem today with java 7.
Hope to see these bugfixes released very soon. (including on your mvn repository ;) ).

Cheers.

Rick Horowitz

unread,
Oct 24, 2012, 6:53:49 PM10/24/12
to Arnaud Mary, twig-p...@googlegroups.com
Just build from the latest sources. It's working.

Cheers!
--
Rick Horowitz
Reply all
Reply to author
Forward
0 new messages