JDK 14 RFR of JDK-8232230: Suppress warnings on non-serializable non-transient instance fields in java.util.concurrent

[Joe Darcy]

Hi Martin,

On 10/14/2019 10:28 PM, Martin Buchholz wrote:
> Hi Joe,
>
> I'm surprised there were so few changes.


The changes in question allow the java.util.concurrent package to pass 
cleanly through the current version of my checks. Perhaps a strong 
version of the checks in the future will flag more issues of possible 
concern ;-)


>
> Why are some of the Condition's marked // Conditionally serializable

Summarizing the discussion leading up to the changes for JDK-8231913, 
"Conditionally serializable" describes collections that are serializable 
iff their contents are serializable. The changes made for

8231202: Suppress warnings on non-serializable non-transient instance 
fields in serializable classes 
http://hg.openjdk.java.net/jdk/jdk/rev/e036ee8bae56

use this terminology in the comments added for many collection types, 
including Vector, TreeMap, and multiple implementation classes in 
java.util.Collections. I tried to follow the same pattern in 
java.util.concurrent where appropriate.

The other comment is a broader statement acknowledging the cause of the 
warning, a non-serializable type as an instance field in a serializable 
class (in the absence of serialPersistentFields, etc.).

> and others // Not statically typed as Serializable
> ?
> ---
> (I don't recall us ever testing serializability of Conditions)
> ---
> ThreadPoolExecutor.Worker already has
>         /**
>          * This class will never be serialized, but we provide a
>          * serialVersionUID to suppress a javac warning.
>          */
>         private static final long serialVersionUID = 6138294804551838833L;
>
> Perhaps that should be replaced with a class-level suppression?

That would be fine with me if that is the preferred option for the JSR 
166-alpha maintainers. However, it would be stronger in that case if in 
addition to the @SuppressWarning("serial") at the class level, 
readObject and writeObject methods that unconditionally threw exceptions 
were added.

For the logistics of getting this change back, would you push the change 
to the java.util.concurrent upstream and then sync it down or should I 
push to jdk/jdk?

Thanks,

-Joe

>
>
> On Mon, Oct 14, 2019 at 5:33 PM Joe Darcy <joe.darcy at oracle.com 
> <mailto:joe.darcy at oracle.com>> wrote:
>
>     Hello,
>
>     Expanding the serialization review to include the
>     java.util.concurrent
>     package, please review the proposed changes:
>
>          JDK-8232230: Suppress warnings on non-serializable non-transient
>     instance fields in java.util.concurrent
>     http://cr.openjdk.java.net/~darcy/8232230.0/
>
>     Terminology added by  JDK-8231913: "Discuss serializability of
>     collections" used where appropriate.
>
>     Thanks,
>
>     -Joe
>