RFR 8159751: ObjectStreamClass: hide 'final' flag for anonymous classes

[Dan Smith]

> On Jun 23, 2016, at 2:04 PM, Roger Riggs <roger.riggs at oracle.com> wrote:
> 
> Hi Dan,
> 
> On 6/23/2016 2:53 PM, Dan Smith wrote:
>>> On Jun 23, 2016, at 11:37 AM, Roger Riggs <roger.riggs at oracle.com>
>>>  wrote:
>>> 
>>> Hi Dan,
>>> 
>>> I was concerned about inter-operation between versions because serialized objects
>>> are passed between Java runtimes with different versions and both need to compute
>>> the same serial version uid (if it is not explicitly declared). The older java runtimes will
>>> compute the serial version uid without regard to your change.
>>> 
>>> If javac always turns on the ACC_FINAL bit and using -target 8 and then the class file
>>> is executed in a Java 8 runtime, it seems like it would compute a different value
>>> which would be in compatible change.  It might be safer to introduce this change
>>> only with the new class file version number.
>>> 
>> Ugh, this is a good point.  Hadn't thought about that use case.
>> 
>> How should we compare these two compatibility risks?:
>> 
>> (1) Using identical class binaries on two sides of a pipe, but running different JDK versions, a class has different UIDs on the two sides.
>> 
> Go back to the rationale for not including ACC_FINAL in the modifiers used in SUID?
> I'm not clear on that part (other than its a current bug, not spec compliant).
> Reflection could report final based on the 'implicitly' final spec for anonymous classes.
> Or reflection needs to be classfile version aware.
> 
> It is simpler to just hash whatever bits are there and not modify them.
> It won't break because of compiler changes or re-compiles.

Rationale: javac would like to change the bits generated for anonymous inner classes -- specifically, by setting the ACC_FINAL bit, where it has been left unset in the past -- but doesn't want to disrupt clients who use serialization.  (Why not?  To be nice, mostly -- trying to avoid (2).)  If we can force these new classes to have the same UID as the old classes, then there's no problem.

Except, oops, we can't redefine the implicit UID definition without introducing interop problems between different JDK versions -- that's (1).

I don't think it's necessarily bad for ObjectStreamClass to manipulate the bits before hashing them -- we already mask out lots of flags, per the serialization spec -- but the problem here is that any change to the hashing behavior (for a class that legally exists with an older class file version) is going to introduce incompatibilities between different versions of the java.io API.

Changing the behavior of reflection doesn't help: you end up with the same problem, a single binary class having different UIDs depending on which version of the Java APIs is being used.  (Besides, using reflection to obfuscate the actual contents of a class file is something we should avoid.)

>> (2) Using class binaries compiled from different versions of javac on two sides of a pipe, but running the same JDK versions, a class has different UIDs on the two sides.
>> 
>> Your suggestion to use "-target" helps to reduce the likelihood of (1).  But there may still be classes that are broken in this scenario.  Some examples:
>> - Really old classes that had ACC_FINAL set (it has been turned on and off once or twice in javac over the years)
>> - Classes that target 9, but a bytecode rewriter converts to run on 8, without tweaking ACC_FINAL
>> 
> Not your problem, different rules apply to different class file versions, tools need to respect/ deal with it.

There are no rules that say that the 'inner_class_access_flags' value for an anonymous inner class (as defined by Class.isAnonymousClass) must be ACC_FINAL, or must not be ACC_FINAL.  Both are totally fine, as far as the JVM is concerned.

The reason we want to change javac's behavior is because the *language specification* says the class is 'final', which ought to constrain the subset of anonymous inner classes that are generated by compilers for the Java language.

>> - Classes generated by other compilers (Eclipse doesn't use ACC_FINAL for anon inner classes at the moment, but what about, e.g., other languages?)
>> 
>> As for (2), fixing this javac bug without touching ObjectStreamClass will impact all Serializable anonymous inner classes -- a much wider net.  Then again, they've received fair warning: "the default serialVersionUID computation is highly sensitive to class details that may vary depending on compiler implementations"; and my understanding of common practice is that users who serialize anonymous classes understand that they should have identical binaries on both sides.
>> 
> And javac warns about missing SerialVersionUID fields with avoids the problem entirely.

Yep, this disclaimer isn't just buried in a spec somewhere -- the compiler actively tells you about it.

> And its not just a javac bug, it appears that class files can vary, either it is allowed or 
> it should be/should have been a verification error.

Not an issue.  Per above, we're not interested in having the JVM restrict what's allowed by 'inner_class_access_flags'.

>> So (1) is a much narrower problem, but also more serious: users can reasonably consider a failure to serialize/deserialize with identical class binaries to be a bug; they can't really complain about a new compiler version spitting out different bytes.
>> 
> I would suggest javac to produce the same ACC_FINAL bits as it did before for each -target version.
> (I know it looks like a hack, but a compatible one).
> Correct it for 9 and leave the SUID computation alone.

Okay, so I interpret this to mean you think (2) is tolerable, at least where the two binaries for the anonymous class have different version numbers?

> Consider if classes should fail verification if the bit is wrong, so 3rd party tools get corrected.

Nope, don't want to change the JVM spec -- it's not "wrong", just not intended javac output.

—Dan