[7u-dev] Request for approval for JDK-7175464 - entrySetView field is never updated in NavigableSubMap

[Peter Levart]

Hi Martin,

It seems that adding a default method Comparator.reverseOrder() had an 
impact on the code in Collections.

In he following code in Collections:

     private static class ReverseComparator
         implements Comparator<Comparable<Object>>, Serializable {

         private static final long serialVersionUID = 7207038068494060240L;

         static final ReverseComparator REVERSE_ORDER
             = new ReverseComparator();

         public int compare(Comparable<Object> c1, Comparable<Object> c2) {
             return c2.compareTo(c1);
         }

         private Object readResolve() { return reverseOrder(); }
     }

...the method readResolve() now calls default Comparator.reverseOrder(), 
but previously it called the static Collections.reverseOrder()....


Regards, Peter

On 02/10/2013 08:49 AM, Martin Buchholz wrote:
> [adding lambda-dev]
>
> Here's another refinement to the test case, which shows that a serial clone
> of Collections.reverseOrder in lambda8 creates a new instance of a new
> class with the opposite order (which I can't explain):
>
> When run against latest lambda-b76, it gives this output:
>
> x=java.util.Collections$ReverseComparator at 3710b205
> y=java.util.Collections$ReverseComparator2 at e9b8b810
> x: 1 -1
> y: -1 1
>
> import java.util.*;
> import java.io.*;
>
> public class ReverseOrder {
>      public static void main(String[] args) throws Throwable {
>          Comparator x = Collections.reverseOrder();
>          Comparator y = serialClone(x);
>          System.out.printf("x=%s%n", x);
>          System.out.printf("y=%s%n", y);
>          System.out.printf("x: %d %d%n", x.compare(0,1), x.compare(1,0));
>          System.out.printf("y: %d %d%n", y.compare(0,1), y.compare(1,0));
>      }
>
>      static byte[] serialBytes(Object o) {
>          try {
>              ByteArrayOutputStream bos = new ByteArrayOutputStream();
>              ObjectOutputStream oos = new ObjectOutputStream(bos);
>              oos.writeObject(o);
>              oos.flush();
>              oos.close();
>              return bos.toByteArray();
>          } catch (Throwable t) {
>              throw new Error(t);
>          }
>      }
>
>      @SuppressWarnings("unchecked")
>      static <T> T serialClone(T o) {
>          try {
>              ObjectInputStream ois = new ObjectInputStream
>                  (new ByteArrayInputStream(serialBytes(o)));
>              T clone = (T) ois.readObject();
>              return clone;
>          } catch (Throwable t) {
>              throw new Error(t);
>          }
>      }
> }
>
>
> On Sat, Feb 9, 2013 at 3:31 PM, Martin Buchholz <martinrb at google.com> wrote:
>
>> On Sat, Feb 9, 2013 at 3:09 PM, Martin Buchholz <martinrb at google.com>wrote:
>>
>>> It looks to me like Collections.reverseOrder no longer deserializes to
>>> the same object.  It also looks like the definition for that in
>>> Collections.java hasn't changed recently.  So I suspect that there has been
>>> some serious incompatible change to deserialization itself.
>>> (It's another matter whether that could break TreeSet).
>>> (I have long lobbied for more cross-jdk testing focused on seriallization)
>>>
>>> The program below demonstrates the different behavior between jdk7 and
>>> jdk8:
>>>
>>>
>> Oops - correction - this is not a difference between jdk7 and jdk8, but
>> between jdk8 and lambda8, More specifically, lambda-8-b74 fails,
>> while jdk8-b74 succeeds.  Have lambdadukes messed with serialization?
>>
>>
>>
>>> import java.util.*;
>>> import java.io.*;
>>>
>>> public class ReverseOrder {
>>>      public static void main(String[] args) throws Throwable {
>>>          Comparator c = Collections.reverseOrder();
>>>          if (c != serialClone(c))
>>>              throw new Error(String.format("c=%s clone=%s",
>>>                                            c, serialClone(c)));
>>>      }
>>>
>>>       static byte[] serialBytes(Object o) {
>>>          try {
>>>              ByteArrayOutputStream bos = new ByteArrayOutputStream();
>>>              ObjectOutputStream oos = new ObjectOutputStream(bos);
>>>              oos.writeObject(o);
>>>              oos.flush();
>>>              oos.close();
>>>              return bos.toByteArray();
>>>          } catch (Throwable t) {
>>>              throw new Error(t);
>>>          }
>>>      }
>>>
>>>      @SuppressWarnings("unchecked")
>>>      static <T> T serialClone(T o) {
>>>          try {
>>>              ObjectInputStream ois = new ObjectInputStream
>>>                  (new ByteArrayInputStream(serialBytes(o)));
>>>              T clone = (T) ois.readObject();
>>>              return clone;
>>>          } catch (Throwable t) {
>>>              throw new Error(t);
>>>           }
>>>      }
>>> }
>>>
>>>
>>> On Fri, Feb 8, 2013 at 3:16 PM, Mike Duigou <mike.duigou at oracle.com>wrote:
>>>
>>>> Thank you for catching this Doug. I missed your original post on this
>>>> topic during my Christmas vacation. (
>>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-December/013127.htmlfor those following along at home)
>>>>
>>>> I will definitely hold off and follow up on the potentially bad patch to
>>>> Java 8. I have created an issue to resolve the test breakage, JDK-8007889
>>>>
>>>> Mike
>>>>
>>>> On Feb 8 2013, at 11:43 , Doug Lea wrote:
>>>>
>>>>> On 02/08/13 14:33, Mike Duigou wrote:
>>>>>> Hello all;
>>>>>>
>>>>>> I would like to backport this change from Java 8. It has been baking
>>>> in JDK8 for about two months with no problems.
>>>>> I think it may have problems.
>>>>> As I mentioned in a post a few months ago, it seems
>>>>> to be responsible for breakage in a TCK/JCK test;
>>>>> One derived from a jsr166 tck test at
>>>>>
>>>> http://gee.cs.oswego.edu/cgi-bin/viewcvs.cgi/jsr166/src/test/tck/TreeSubSetTest.java?view=log
>>>>> You need the file for context, but it looks like:
>>>>>
>>>>>     public void testDescendingSerialization() throws Exception {
>>>>>         NavigableSet x = dset5();
>>>>>         NavigableSet y = serialClone(x);
>>>>>
>>>>>         assertTrue(x != y);
>>>>>         assertEquals(x.size(), y.size());
>>>>>         assertEquals(x.toString(), y.toString());
>>>>>         assertEquals(x, y);
>>>>>         assertEquals(y, x);
>>>>>         while (!x.isEmpty()) {
>>>>>             assertFalse(y.isEmpty());
>>>>>             assertEquals(x.pollFirst(), y.pollFirst());
>>>>>         }
>>>>>         assertTrue(y.isEmpty());
>>>>>     }
>>>>>
>>>>>
>>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7175464
>>>>>>
>>>>>> http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/bf6ceb6b8f80
>>>>>>
>>>>>> The change was previously reviewed by Alan Bateman, Paul Sandoz and
>>>> David Holmes before going in to Java 8.
>>>>>> Mike
>>>>>>