JDK 14 RFR of JDK-8231202: Suppress warnings on non-serializable non-transient instance fields in serializable classes
forax at univ-mlv.fr
Thu Sep 19 20:45:29 UTC 2019
----- Mail original -----
> De: "joe darcy" <joe.darcy at oracle.com>
> À: "Roger Riggs" <Roger.Riggs at oracle.com>, "core-libs-dev" <core-libs-dev at openjdk.java.net>
> Envoyé: Jeudi 19 Septembre 2019 21:39:54
> Objet: Re: JDK 14 RFR of JDK-8231202: Suppress warnings on non-serializable non-transient instance fields in
> serializable classes
> Hi Roger,
> I think the Object -> Serializable type changes you suggest are fine
> Would you like to push them first ahead of applying @SuppressWarnings to
> the remaining locations?
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).
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).
- 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.
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,
or said differently if the serialization was introduced after Java 5, is Serializable still be an interface or be an annotation instead.
> 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
>> the fix should be to correct the types used.
>> For example, in the serialization proxies for java.time, the Ser class
>> 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:
>>> 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
>>> 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
>>> 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.
More information about the core-libs-dev