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.da...@oracle.com
<mailto:joe.da...@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