On Wed, 30 Jul 2025 03:44:37 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org>
wrote:
>> src/java.desktop/share/classes/javax/swing/JPopupMenu.java line 1427:
>>
>>> 1425: indexCounter++;
>>> 1426: }
>>> 1427: }
>>
>> This may be a naive question: is the lambda we use for `propListener`
>> Serializable?
>>
>> (If so: great. If not: won't we get inconsistent results if we're working
>> with a deserialized copy of a JPopupMenu? The original will have a non-null
>> `propListener` field, and the copy will have a null `propListener`? Or am I
>> missing something?)
>
> It is transient so its value should not be included in the default
> serialization process in my opinion
I'm fine with `propListener` being transient ... as long as when we call
readObject() we still make a point to initialize `propListener`.
Here's a unit test that demonstrates what I'm worried about:
import javax.swing.*;
import java.beans.PropertyChangeListener;
import java.io.*;
public class JPopupMenuSerializationTest {
static class JPopupMenuSubclass extends JPopupMenu implements Serializable {
private transient PropertyChangeListener propListener =
(e) -> {
if (e.getOldValue() != null
&& e.getNewValue() == null
&& isVisible()) {
setVisible(false);
}
};
@Serial
private void writeObject(ObjectOutputStream s) throws IOException {
Object serializable = propListener instanceof Serializable ?
(Serializable) propListener : null;
System.out.println("Serialized: " + serializable);
s.writeObject(serializable);
}
@Serial
private void readObject(ObjectInputStream s)
throws IOException, ClassNotFoundException {
PropertyChangeListener l = (PropertyChangeListener) s.readObject();
System.out.println("Deserialized: " + l);
if (l != null) {
propListener = l;
}
System.out.println("propListener: " + propListener);
}
}
public static void main(String[] args) throws Exception {
JPopupMenuSerializationTest test = new JPopupMenuSerializationTest();
test.testSerialization();
}
public void testSerialization() throws Exception {
JPopupMenuSubclass popupMenu = new JPopupMenuSubclass();
byte[] data = serialize(popupMenu);
JPopupMenuSubclass copy = deserialize(data);
assertTrue("original.propListener should exist", popupMenu.propListener
!= null);
// currently this fails, and IMO it should pass:
assertTrue("copy.propListener should exist", copy.propListener !=
null);
}
private static void assertTrue(String errorMsg, boolean b) {
if (!b)
throw new Error(errorMsg);
}
private static byte[] serialize(JPopupMenuSubclass popupMenu) throws
IOException {
try (ByteArrayOutputStream byteOut = new ByteArrayOutputStream()) {
try (ObjectOutputStream objOut = new ObjectOutputStream(byteOut)) {
objOut.writeObject(popupMenu);
}
return byteOut.toByteArray();
}
}
private static JPopupMenuSubclass deserialize(byte[] data) throws
IOException, ClassNotFoundException {
try (ByteArrayInputStream byteIn = new ByteArrayInputStream(data)) {
try (ObjectInputStream objIn = new ObjectInputStream(byteIn)) {
return (JPopupMenuSubclass) objIn.readObject();
}
}
}
}
Currently on my machine this outputs:
Serialized: null
Deserialized: null
propListener: null
Exception in thread "main" java.lang.Error: copy.propListener should exist
at
JPopupMenuSerializationTest.assertTrue(JPopupMenuSerializationTest.java:54)
at
JPopupMenuSerializationTest.testSerialization(JPopupMenuSerializationTest.java:49)
at JPopupMenuSerializationTest.main(JPopupMenuSerializationTest.java:38)
This means (I think?) a deserialized copy is going to act different than the
original, right? (I'm not an expert on serializing UI components, so maybe I'm
missing something...?)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26407#discussion_r2241513779
