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

[Roger Riggs]

Hi Rémi,

On 9/19/19 4:45 PM, Remi Forax wrote:
>
> Hi Joe, hi Roger,
>
> these changes are not fine, SerializedLambda is a public class with a public constructor so it's an incompatible change to replace Object[] by Serializable[],
> moreover for an array, all values can be Serializable but the array itself can be not Serializable, by example if the array is created as a Object... (varargs).
Correct, this change is incompatible; withdrawn.

All arrays are Serializable even if the array type does not 'implement 
Serializable'.
But the static type checks can't help with that.
>
> there are other issues:
> - a class can be marked as Serializable but not being serialized at runtime, in that case, it's valid to construct an instance of a Serializable class with an argument which is typed Object instead of Serializable because it will not serialized (basically Serializable means can be serialized and not will be serialized).
Right, I would not suggest using a static Serializable type in that case.
Generally, I view using Object as a undesirable shortcut.
> - using Serializable as type of a field means that if you have an interface you have to introduce a cast somewhere in the code, so you are moving the CCE that comes from writeObject to a CCE in the middle of the user code, i think i prefer the CCE to be issued inside writeObject because at least it's documented.
For the java.time.*.Ser suggestions, all of the objects implement 
Serializable
and the Ser class is an implementation artifact only supporting 
Serialization.
With the aid of sealing, the CCE should not need to be a runtime check.
>
> the real question is what are the pro and cons of Serializable acting as a type instead of only as a marker interface that you can check at runtime,
Serializable is a real type and the code better reflects the typing 
requirements than Object.

> or said differently if the serialization was introduced after Java 5, is Serializable still be an interface or be an annotation instead.
Regrettable that we didn't understand 20 years ago what we understand today
and had all of the language and runtime features available now. Sigh...

Thanks, Roger

>
> Rémi
>
>> 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
>>>>