From 55ae24c293513cd57a3b1392ccd4a8f3afeae933 Mon Sep 17 00:00:00 2001 From: Erik Measure Date: Sun, 24 Mar 2024 07:41:17 -0700 Subject: [PATCH 1/4] Fix concurrency bug --- cryptography-providers/jdk/src/jvmMain/kotlin/pooling.kt | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/cryptography-providers/jdk/src/jvmMain/kotlin/pooling.kt b/cryptography-providers/jdk/src/jvmMain/kotlin/pooling.kt index d9f33285..e98111d3 100644 --- a/cryptography-providers/jdk/src/jvmMain/kotlin/pooling.kt +++ b/cryptography-providers/jdk/src/jvmMain/kotlin/pooling.kt @@ -16,11 +16,9 @@ internal sealed class Pooled(protected val instantiate: () -> T) { private val pooled = ArrayDeque() override fun get(): T { - synchronized(this) { - pooled.firstOrNull() - }?.let { return it } - - return instantiate() + return synchronized(this) { + pooled.removeLastOrNull() + } ?: instantiate() } override fun put(value: T) { From 54d5a9ca23522f9d568241f43d3daa0542881a3d Mon Sep 17 00:00:00 2001 From: Erik Measure Date: Sun, 24 Mar 2024 13:11:25 -0700 Subject: [PATCH 2/4] Add unit test --- .../jdk/src/jvmTest/kotlin/PoolingTest.kt | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt diff --git a/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt b/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt new file mode 100644 index 00000000..f60ffd63 --- /dev/null +++ b/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt @@ -0,0 +1,33 @@ +package dev.whyoleg.cryptography.providers.jdk + +import kotlinx.coroutines.* +import kotlinx.coroutines.test.* +import kotlin.test.* + +class PoolingTest { + + @Test + fun testSequentialAccessOnCachedPoolShouldReuseInstance() { + var instantiateCount = 0 + val pool = Pooled.Cached { instantiateCount++; Any() } + repeat(3) { + pool.use { } + } + assertEquals(1, instantiateCount) + } + + @Test + fun testConcurrentAccessOnCachedPoolShouldNotReuseInstances() = runTest { + var instantiateCount = 0 + val pool = Pooled.Cached { instantiateCount++; Any() } + pool.use { } // prime the pool with 1 instance; we should not be able to reuse that instance for all 3 concurrent usages below + List(3) { + launch { + pool.use { + delay(1000) + } + } + }.joinAll() + assertEquals(3, instantiateCount) + } +} From 561fafe17cdb0c54521c8f2e87bff1a653d860eb Mon Sep 17 00:00:00 2001 From: Erik Measure Date: Mon, 25 Mar 2024 18:19:01 -0700 Subject: [PATCH 3/4] Add unit test --- .../jdk/src/jvmTest/kotlin/PoolingTest.kt | 47 +++++++++++++------ 1 file changed, 32 insertions(+), 15 deletions(-) diff --git a/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt b/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt index f60ffd63..e9c67694 100644 --- a/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt +++ b/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt @@ -7,27 +7,44 @@ import kotlin.test.* class PoolingTest { @Test - fun testSequentialAccessOnCachedPoolShouldReuseInstance() { - var instantiateCount = 0 - val pool = Pooled.Cached { instantiateCount++; Any() } - repeat(3) { - pool.use { } + fun testSequentialUseCachedPool() { + val pool = Pooled.Cached(::Any) + val first = pool.use { it } + val second = pool.use { it } + assertSame(first, second) + val third = pool.use { it } + assertSame(second, third) + } + + @Test + fun testOverlappingUseCachedPool() = runTest { + val pool = Pooled.Cached(::Any) + val first = pool.use { it } + pool.use { i1 -> + assertSame(first, i1) + pool.use { i2 -> + assertNotSame(i1, i2) + pool.use { i3 -> + assertNotSame(i2, i3) + } + } } - assertEquals(1, instantiateCount) } @Test - fun testConcurrentAccessOnCachedPoolShouldNotReuseInstances() = runTest { - var instantiateCount = 0 - val pool = Pooled.Cached { instantiateCount++; Any() } - pool.use { } // prime the pool with 1 instance; we should not be able to reuse that instance for all 3 concurrent usages below - List(3) { - launch { - pool.use { + fun testConcurrentUseCachedPool() = runTest { + val pool = Pooled.Cached(::Any) + val first = pool.use { it } + val instances = List(3) { + async { + pool.use { instance -> delay(1000) + instance } } - }.joinAll() - assertEquals(3, instantiateCount) + }.awaitAll() + assertSame(first, instances[0]) + assertNotSame(instances[0], instances[1]) + assertNotSame(instances[1], instances[2]) } } From ed262f6958414eacc68b85077478a2d79deda704 Mon Sep 17 00:00:00 2001 From: Erik Measure Date: Mon, 25 Mar 2024 18:34:56 -0700 Subject: [PATCH 4/4] Remove extraneous runTest --- cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt b/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt index e9c67694..2276a9b7 100644 --- a/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt +++ b/cryptography-providers/jdk/src/jvmTest/kotlin/PoolingTest.kt @@ -17,7 +17,7 @@ class PoolingTest { } @Test - fun testOverlappingUseCachedPool() = runTest { + fun testOverlappingUseCachedPool() { val pool = Pooled.Cached(::Any) val first = pool.use { it } pool.use { i1 ->