Bug 339447 - synchronized access modifier retained on clone() bridge
Summary: synchronized access modifier retained on clone() bridge
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 M7   Edit
Assignee: Olivier Thomann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-03-09 18:48 EST by Tim Eck CLA
Modified: 2011-04-25 13:54 EDT (History)
4 users (show)

See Also:
srikanth_sankaran: review+


Attachments
Proposed fix + regression test (4.81 KB, patch)
2011-03-15 11:58 EDT, Olivier Thomann CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Eck CLA 2011-03-09 18:48:46 EST
Build Identifier: M20110210-1200

public class Foo implements Cloneable {
    public synchronized Foo clone() {
        return this;
    }    
}

Compile this class with the eclipse compiler (default 1.6 compliance settings). Also compile the same class with javac from hotspot 1.6.0_24. 

In the resulting class files the bridge method for clone() ends up with different access modifiers. The eclipse compiler keeps the synchronized access where as javac does not. 

For the most part this difference doesn't really matter but it was biting me since the different modifiers leads to a different serialVersionUID and breaks serialization. 


Reproducible: Always
Comment 1 Tim Eck CLA 2011-03-09 18:50:07 EST
The javap output (from eclipse .class)
Compiled from "Foo.java"
public class Foo extends java.lang.Object implements java.lang.Cloneable{
public Foo();
  Code:
   0:   aload_0
   1:   invokespecial   #10; //Method java/lang/Object."<init>":()V
   4:   return

public synchronized Foo clone();
  Code:
   0:   aload_0
   1:   areturn

public synchronized java.lang.Object clone()   throws java.lang.CloneNotSupportedException;
  Code:
   0:   aload_0
   1:   invokevirtual   #22; //Method clone:()LFoo;
   4:   areturn

}


And the output from the javac compiler one (note: clone() bridge is not synchronized access):

Compiled from "Foo.java"
public class Foo extends java.lang.Object implements java.lang.Cloneable{
public Foo();
  Code:
   0:   aload_0
   1:   invokespecial   #1; //Method java/lang/Object."<init>":()V
   4:   return

public synchronized Foo clone();
  Code:
   0:   aload_0
   1:   areturn

public java.lang.Object clone()   throws java.lang.CloneNotSupportedException;
  Code:
   0:   aload_0
   1:   invokevirtual   #2; //Method clone:()LFoo;
   4:   areturn

}
Comment 2 Olivier Thomann CLA 2011-03-09 20:55:33 EST
(In reply to comment #0)
> For the most part this difference doesn't really matter but it was biting me
> since the different modifiers leads to a different serialVersionUID and breaks
> serialization. 
There are multiple reasons that will lead to a different serialVersionUID from the javac's one. If you really care about serialization, you should set your own serialVersionUID.
This being said, we will check what is supposed to be done regarding the synchronized modifiers and bridge method.
Comment 3 Tim Eck CLA 2011-03-10 02:19:37 EST
Thanks for the input Oliver. For sure the best thing to do for serialization is to be explicit in the code for serialVersionUID. 

I have no idea if this particular detail is covered in the specs. That said it does seem unnecessary to carry synchronized access in the bridge method so I lean towards javac being more "correct" in this case
Comment 4 Olivier Thomann CLA 2011-03-10 09:42:54 EST
(In reply to comment #3)
> Thanks for the input Oliver. For sure the best thing to do for serialization is
> to be explicit in the code for serialVersionUID. 
This is actually the only way to do it to get a reliable behavior. javac and the eclipse compiler don't handle the class literal (X.class for example) the same way. So any usage of the class literal in source would lead to a different serialVersionUID.

> I have no idea if this particular detail is covered in the specs. That said it
> does seem unnecessary to carry synchronized access in the bridge method so I
> lean towards javac being more "correct" in this case.
I'll double-check that.
Comment 5 Olivier Thomann CLA 2011-03-15 11:44:55 EDT
I could not find any specific details on the synchronized keyword, but I agree that it makes sense to drop it since the "inner" call is synchronized.
Srikanth, any comment on this ?
Comment 6 Olivier Thomann CLA 2011-03-15 11:58:15 EDT
Created attachment 191223 [details]
Proposed fix + regression test
Comment 7 Olivier Thomann CLA 2011-03-15 11:58:40 EDT
Srikanth, please review. Fix is trivial.
Comment 8 Srikanth Sankaran CLA 2011-03-16 01:31:25 EDT
Agree with the fix. Patch looks good.
Comment 9 Olivier Thomann CLA 2011-03-18 09:20:10 EDT
Released for 3.7M7.
Comment 10 Jay Arthanareeswaran CLA 2011-04-25 13:54:57 EDT
Verified for 3.7M7 using build I20110421-1800