altoo-ag/akka-kryo-serialization

Concurrent deserialization of Collections of Enums leads to nulls and duplicated Enumeration#Values

Closed this issue · 5 comments

Test case:

  case class Bar(time:Time)

  "Enumeration deserialization" should "be threadsafe" in {
    import scala.concurrent.ExecutionContext.Implicits.global

    val defaultConfig = ConfigFactory.parseString("""
    akka {
      extensions = ["com.romix.akka.serialization.kryo.KryoSerializationExtension$"]
      actor {
        serializers {
          kryo         = "com.romix.akka.serialization.kryo.KryoSerializer"
        }
        serialization-bindings {
          "java.io.Serializable"        = kryo
        }
        kryo  {
          idstrategy  = "default"
        }
      }
    }
    """)
    val system = ActorSystem("testSystem", defaultConfig)
    val serialization = SerializationExtension(system)

    val obuf1 = new Output(1024, 1024 * 1024)

    val listOfBars = Time.values.map(Bar(_)).toList
    val bytes = serialization.serialize(listOfBars).get

    val futures = 1 to 2 map(_ => Future[List[Bar]] {
      serialization.deserialize(bytes.clone, classOf[List[Bar]]).get
    })

    val result = Await.result(Future.sequence(futures), Duration.Inf)
    assert(result.map(_.map(_.time)).flatten.distinct.size == Time.values.size)
  }

Output:

Vector(Second, Month, Minute, Day, Year, Hour, Second, null) had size 8 instead of expected size 6
luben commented

Hi,

I am trying to reproduce it, what is the "Time" type?

Regards,

luben commented

Aaa, I found it

luben commented

@stefan-mees can you try with the PR I just sent, it should fix the issue. Thanks for the test case, I have also included it.

Commented on the PR, looks good so far - but as far as i can see, Kryo will also create new instances which should be avoided.

luben commented

Yes, I shot down the previous PR because of that reason. The new PR is not creating new instances for each deserialized value. It also fixes the concurrency issue.