JDK 14 RFR of JDK-8231202: Suppress warnings on non-serializable non-transient instance fields in serializable classes

[Joe Darcy]

Hi Roger,

I think the Object -> Serializable type changes you suggest are fine 
improvements.

Would you like to push them first ahead of applying @SuppressWarnings to 
the remaining locations?

Thanks,

-Joe

On 9/19/2019 12:13 PM, Roger Riggs wrote:
> Hi Joe,
>
> The addition of @SuppressWarnings(serial) hides a number of instances of
> poor choices of types.  Before they dissappear behind the suppress 
> warnings,
> the fix should be to correct the types used.
>
> For example, in the serialization proxies for java.time, the Ser class 
> carelessly
> has a field of type Object when it could/should be using Serializable.
> The types being serialized and deserialized are known to be Serializable.
> See the attach patches for details and a suggested fix.
>
> Thanks, Roger
>
> (p.s. the patch is attached twice, once as .patch and the other .txt.
> I'd like to see if they both get through the mail).
>
> On 9/18/19 5:38 PM, Joe Darcy wrote:
>> Hello,
>>
>> As background, I'm working on a number of serialization-related 
>> compile-time checks with the goal of enabling stricter javac lint 
>> checking in the JDK build (and elsewhere).
>>
>> One check is tracked by
>>
>>     JDK-8160675: Issue lint warning for non-serializable 
>> non-transient instance fields in serializable type
>>
>> As summarized in the bug description, it may be concerning if a 
>> serializable class has non-transient instance fields whose types are 
>> not Serializable. This can cause a serialization failure at runtime. 
>> (Classes using the serialPersistentFields mechanism are excluded from 
>> this check.)
>>
>> A common example is an exception type -- all Throwable's are 
>> Serializable -- which has a non-serializable field. If the fields 
>> cannot be marked as transient, one approach to handle this robustly 
>> is to have a writeObject method which null outs the field in question 
>> when serializing and make the other methods in the exception 
>> null-tolerant.
>>
>> In other cases, the object pointed to by the non-serializable field 
>> are conditionally serializable at runtime. This is the case for many 
>> collection types. For example, a class may have a field of type 
>> List<Foo> with the field set to an ArrayList<Foo> at runtime. While 
>> the List interface does not extent Serializable, the ArrayList class 
>> does implement Serializable and the class would serialize fine in 
>> practice, assuming the Foo's were serialazable.
>>
>> As a precursor to the adding a compile-time check to the build, 
>> please review adding @SuppressWarnings("serial") to document the 
>> non-serializable fields in the core libraries:
>>
>>     JDK-8231202: Suppress warnings on non-serializable non-transient 
>> instance fields in serializable classes
>> http://cr.openjdk.java.net/~darcy/8231202.0/
>>
>> Bugs for similar changes to client libs and security libs will be 
>> filed and reviewed separately.
>>
>> A more complete fix would add readObject/writeObject null handling to 
>> AnnotationTypeMismatchExceptionProxy, but since this hasn't seemed to 
>> be an issue since the type was introduced back in JDK 5.0, I just 
>> added the annotation, as done elsewhere.
>>
>> Thanks,
>>
>> -Joe
>>
>