RFR [9] 8071472: Add field access to support setting final fields in readObject

[Peter Levart]

On 03/17/2015 11:39 AM, Alan Bateman wrote:
> On 17/03/2015 09:57, Chris Hegarty wrote:
>> Peter, Alan,
>>
>> After further thought I think that requiring all final fields to be 
>> set is reasonable behaviour. Since there is no compile time checking, 
>> this is a reasonable runtime effort to catch what is likely to be 
>> developer errors. To address Alan’s comments, I beefed up the API 
>> docs and added examples to typical usage.
>>
>> Updated specdiff (and webrev):
>> http://cr.openjdk.java.net/~chegar/8071472/01/ 
>> <http://cr.openjdk.java.net/%7Echegar/8071472/01/>
>>
>> This version also includes a number of changes to readObject 
>> implementations in the base module, to replace unsafe usage with this 
>> field setter API.
>>
>>
> The rename to FieldSetter looks good, also good to have an example in 
> the javadoc.
>
> It is good to catch cases where final fields aren't set but the issue 
> is the surprising behavior that the check is tied to whether the 
> FieldSetter has been obtained or not. The other thing is that 
> fieldSetter's javadoc doesn't make this clear, you need to read 
> FieldSetter's javadoc to learn about this enforcement. It's hard to 
> know what the right thing to do here as ideally this enforcement would 
> be enabled by default. If my class with final fields has an empty 
> readObject, or it has code paths that don't call defaultReadObject for 
> example, then it's a bug that I'd like to know about. I realize there 
> is potential compatibility, maybe performance, impact of doing this 
> but it does feel like something that I should be able to opt-in or get 
> for free rather than it being a side effect.

Hi Alan,

I agree that not calling defaultReadObject() from readObject() and 
having a final field is potentially a bug. But need not be in case some 
other means of setting final fields was used (Unsafe or reflection). 
Some readObject() implementations in base module that Chris changed to 
use new API fall into this category. We can't track those usages, so to 
keep backwards compatibility, this checking has to be opt-in. Is there a 
more elegant way to opt-in? A @CheckFinalsAssignment annotation on the 
readObject() method?

Regards,

Peter

>
> One other thing that I wonder about is the exception and whether it 
> might be better to bend InvalidObjectException instead.
>
> -Alan
>