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
