Kotlin / kotlinx.collections.immutable

Immutable persistent collections for Kotlin
Apache License 2.0
1.12k stars 56 forks source link

concurrentList.toPersistentList or concurrentMap.toPersistentMap is incorrectly implemented #155

Open develar opened 8 months ago

develar commented 8 months ago

The following code is used by SmallPersistentVector to create persistent list by some collection.

      val newBuffer = buffer.copyOf(size + elements.size)
            // TODO: investigate performance of elements.toArray + copyInto
            var index = size
            for (element in elements) {
                newBuffer[index++] = element
            }
            return SmallPersistentVector(newBuffer)

The issue is that concurrent collections in Java are weakly consistent. So, you create a buffer with a size that is immediately incorrect.

So, you have weird runtime bugs like NPE or array index issues.

As the project uses Gradle, I am not yet able to manage to open it in IDEA and write a test.

JDK (List.copyOf) uses toArray ((List<E>)List.of(coll.toArray())), which is a safe and efficient solution.

qurbonzoda commented 7 months ago

Hi, Thanks for reporting the issue. The library does not prioritise correct handling of concurrent collections. So it becomes the user responsibility to make sure the elements collection consistency. It might make sense to pass a snapshot of your concurrent collection to the function instead.