hrldcpr / pcollections

A Persistent Java Collections Library
Other
765 stars 79 forks source link

HashTreePSet not serializable #79

Closed JimN-LiveAction closed 4 years ago

JimN-LiveAction commented 4 years ago

HashTreePSet is not serializable. Only tested on HashTreePSet.empty(). Tested on pcollections 3.1.0.

Simple test case:

import org.pcollections.HashTreePSet;
import org.pcollections.PSet;

import java.io.ByteArrayOutputStream;
import java.io.ObjectOutputStream;

public class SerializePSet {

  public static void main(String... args) throws Exception {
    PSet set = HashTreePSet.empty();
    ByteArrayOutputStream bytes = new ByteArrayOutputStream();
    ObjectOutputStream out = new ObjectOutputStream(bytes);
    out.writeObject(set);
  }

}

Output:

Exception in thread "main" java.io.NotSerializableException: org.pcollections.IntTreePMap$1
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1184)
    at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1548)
    at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1509)
    at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1178)
    at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1548)
    at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1509)
    at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1178)
    at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1548)
    at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1509)
    at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1178)
    at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:348)
    at test.SerializePSet.main(SerializePSet.java:28)
JimN-LiveAction commented 4 years ago

I see a very similar problem for TreePVector:

Exception in thread "main" java.io.NotSerializableException: org.pcollections.IntTreePMap$1
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1184)
    at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1548)
    at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1509)
    at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1178)
    at java.io.ObjectOutputStream.defaultWriteFields(ObjectOutputStream.java:1548)
    at java.io.ObjectOutputStream.writeSerialData(ObjectOutputStream.java:1509)
    at java.io.ObjectOutputStream.writeOrdinaryObject(ObjectOutputStream.java:1432)
    at java.io.ObjectOutputStream.writeObject0(ObjectOutputStream.java:1178)
    at java.io.ObjectOutputStream.writeObject(ObjectOutputStream.java:348)

This is failing with a vector of our application objects (of size 4). I tried to make a reproducible test case with simple objects (i.e., Integer) but can't reproduce. Not being familiar with pcollections internals, I don't know if the issue is caused by the order/structure of construction or somehow data related such as dependent on Object.hashCode().

Maaartinus commented 4 years ago

FWIW org.pcollections.IntTreePMap$1 is probably the first anonymous class and the proper fix is probably making the field transient.

hrldcpr commented 4 years ago

Thanks for the report! I reproduced the error, putting together a fix now…

hrldcpr commented 4 years ago

Committed a fix. You were right about the need for transient, and in fact I fixed almost this exact bug a year ago in 01f38f9 🤦‍♂️ …this time however I searched for other similar cache variables and found none, so hopefully that's the last of this particular bug.

I'm pushing a new maven release now, usually takes like 6 hours to propagate for some reason.

hrldcpr commented 4 years ago

Fix is live in v3.1.1

Thanks again for the report and solution!

JimN-LiveAction commented 4 years ago

I verified this issue is fixed for me. Thanks for the extremely quick resolution and patch release!