From 45cdee0fb963f6f61395dd0587db3871c1399e60 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Fri, 14 May 2021 17:20:04 +0300 Subject: [PATCH 1/4] Make DescriptorSchemaCache in Json thread-local on Native to avoid freezing problems in custom Json instances Fixes #1450 --- .../src/kotlinx/serialization/json/Json.kt | 7 +++- .../kotlinx/serialization/json/schemaCache.kt | 9 +++++ .../kotlinx/serialization/json/schemaCache.kt | 9 +++++ .../kotlinx/serialization/json/schemaCache.kt | 16 ++++++++ .../serialization/json/MultiWorkerJsonTest.kt | 39 +++++++++++++++++++ 5 files changed, 79 insertions(+), 1 deletion(-) create mode 100644 formats/json/jsMain/src/kotlinx/serialization/json/schemaCache.kt create mode 100644 formats/json/jvmMain/src/kotlinx/serialization/json/schemaCache.kt create mode 100644 formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt create mode 100644 formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt index 1515304d80..f27af16be2 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt @@ -56,7 +56,7 @@ public sealed class Json( override val serializersModule: SerializersModule ) : StringFormat { - internal val schemaCache: DescriptorSchemaCache = DescriptorSchemaCache() + internal val _schemaCache: DescriptorSchemaCache = DescriptorSchemaCache() /** * The default instance of [Json] with default configuration. @@ -291,5 +291,10 @@ private class JsonImpl(configuration: JsonConfiguration, module: SerializersModu } } +/** + * Emulate thread-locality of cache on Native + */ +internal expect val Json.schemaCache: DescriptorSchemaCache + private const val defaultIndent = " " private const val defaultDiscriminator = "type" diff --git a/formats/json/jsMain/src/kotlinx/serialization/json/schemaCache.kt b/formats/json/jsMain/src/kotlinx/serialization/json/schemaCache.kt new file mode 100644 index 0000000000..d303de8ebf --- /dev/null +++ b/formats/json/jsMain/src/kotlinx/serialization/json/schemaCache.kt @@ -0,0 +1,9 @@ +/* + * Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.serialization.json + +import kotlinx.serialization.json.internal.* + +internal actual val Json.schemaCache: DescriptorSchemaCache get() = this._schemaCache diff --git a/formats/json/jvmMain/src/kotlinx/serialization/json/schemaCache.kt b/formats/json/jvmMain/src/kotlinx/serialization/json/schemaCache.kt new file mode 100644 index 0000000000..d303de8ebf --- /dev/null +++ b/formats/json/jvmMain/src/kotlinx/serialization/json/schemaCache.kt @@ -0,0 +1,9 @@ +/* + * Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.serialization.json + +import kotlinx.serialization.json.internal.* + +internal actual val Json.schemaCache: DescriptorSchemaCache get() = this._schemaCache diff --git a/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt b/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt new file mode 100644 index 0000000000..46552cf01b --- /dev/null +++ b/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt @@ -0,0 +1,16 @@ +/* + * Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.serialization.json + +import kotlinx.serialization.json.internal.* + +@ThreadLocal +private val jsonToCache: MutableMap = mutableMapOf() + +/** + * Emulate thread locality of cache on Native + */ +internal actual val Json.schemaCache: DescriptorSchemaCache + get() = jsonToCache.getOrPut(this) { DescriptorSchemaCache() } diff --git a/formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt b/formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt new file mode 100644 index 0000000000..8c45bfc186 --- /dev/null +++ b/formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt @@ -0,0 +1,39 @@ +/* + * Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license. + */ + +package kotlinx.serialization.json + +import kotlinx.serialization.* +import kotlin.native.concurrent.* +import kotlin.test.* + +class MultiWorkerJsonTest { + @Serializable + data class PlainOne(val one: Int) + + @Serializable + data class PlainTwo(val two: Int) + + + @Test + fun testJsonIsFreezeSafe() { + val json = Json { + isLenient = true + ignoreUnknownKeys = true + useAlternativeNames = true + } + val worker = Worker.start() + val operation = { + for (i in 0..999) { + assertEquals(PlainOne(42), json.decodeFromString("""{"one":42,"two":239}""")) + } + } + worker.executeAfter(1000, operation.freeze()) + for (i in 0..999) { + assertEquals(PlainTwo(239), json.decodeFromString("""{"one":42,"two":239}""")) + } + worker.requestTermination() + // Should be completed successfully + } +} From 5029c48a100248bb535f7a1837cf4590143cca68 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Mon, 17 May 2021 17:20:59 +0300 Subject: [PATCH 2/4] ~implement proper deletion --- .../kotlinx/serialization/json/schemaCache.kt | 41 ++++++++++++++++++- .../serialization/json/MultiWorkerJsonTest.kt | 36 +++++++++++----- 2 files changed, 65 insertions(+), 12 deletions(-) diff --git a/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt b/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt index 46552cf01b..f61580de27 100644 --- a/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt +++ b/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt @@ -5,12 +5,49 @@ package kotlinx.serialization.json import kotlinx.serialization.json.internal.* +import kotlin.native.ref.* +import kotlin.random.* @ThreadLocal -private val jsonToCache: MutableMap = mutableMapOf() +private val jsonToCache: MutableMap = mutableMapOf() + +/** + * Because WeakReference itself does not have equals/hashCode + */ +private class WeakJson(json: Json) { + private val ref = WeakReference(json) + private val initialHashCode = json.hashCode() + + val isDead: Boolean get() = ref.get() == null + + override fun equals(other: Any?): Boolean { + if (other !is WeakJson) return false + val thiz = this.ref.get() ?: return false + val that = other.ref.get() ?: return false + return thiz == that + } + + override fun hashCode(): Int = initialHashCode + +} + +/** + * To maintain O(1) access, we cleanup the table from dead references with 1/size probability + */ +private fun cleanUpWeakMap() { + val size = jsonToCache.size + if (size <= 10) return // 10 is arbitrary small number to ignore polluting + // Roll 1/size probability + if (Random.nextInt(0, size) == 0) { + val iter = jsonToCache.iterator() + while (iter.hasNext()) { + if (iter.next().key.isDead) iter.remove() + } + } +} /** * Emulate thread locality of cache on Native */ internal actual val Json.schemaCache: DescriptorSchemaCache - get() = jsonToCache.getOrPut(this) { DescriptorSchemaCache() } + get() = jsonToCache.getOrPut(WeakJson(this)) { DescriptorSchemaCache() }.also { cleanUpWeakMap() } diff --git a/formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt b/formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt index 8c45bfc186..2ea063db6f 100644 --- a/formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt +++ b/formats/json/nativeTest/src/kotlinx/serialization/json/MultiWorkerJsonTest.kt @@ -15,6 +15,20 @@ class MultiWorkerJsonTest { @Serializable data class PlainTwo(val two: Int) + private fun doTest(json: () -> Json) { + val worker = Worker.start() + val operation = { + for (i in 0..999) { + assertEquals(PlainOne(42), json().decodeFromString("""{"one":42,"two":239}""")) + } + } + worker.executeAfter(1000, operation.freeze()) + for (i in 0..999) { + assertEquals(PlainTwo(239), json().decodeFromString("""{"one":42,"two":239}""")) + } + worker.requestTermination() + } + @Test fun testJsonIsFreezeSafe() { @@ -23,17 +37,19 @@ class MultiWorkerJsonTest { ignoreUnknownKeys = true useAlternativeNames = true } - val worker = Worker.start() - val operation = { - for (i in 0..999) { - assertEquals(PlainOne(42), json.decodeFromString("""{"one":42,"two":239}""")) + // reuse instance + doTest { json } + } + + @Test + fun testJsonInstantiation() { + // create new instanceEveryTime + doTest { + Json { + isLenient = true + ignoreUnknownKeys = true + useAlternativeNames = true } } - worker.executeAfter(1000, operation.freeze()) - for (i in 0..999) { - assertEquals(PlainTwo(239), json.decodeFromString("""{"one":42,"two":239}""")) - } - worker.requestTermination() - // Should be completed successfully } } From d22c230a431826b720a547cf0ab8dee7fb69c1a2 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Tue, 18 May 2021 20:28:17 +0300 Subject: [PATCH 3/4] ~ Add docs, rename files, add deprecation for member --- .../src/kotlinx/serialization/json/Json.kt | 7 ++++++- .../json/{schemaCache.kt => JsonSchemaCache.kt} | 1 + .../json/{schemaCache.kt => JsonSchemaCache.kt} | 1 + .../json/{schemaCache.kt => JsonSchemaCache.kt} | 16 +++++++++++++--- 4 files changed, 21 insertions(+), 4 deletions(-) rename formats/json/jsMain/src/kotlinx/serialization/json/{schemaCache.kt => JsonSchemaCache.kt} (90%) rename formats/json/jvmMain/src/kotlinx/serialization/json/{schemaCache.kt => JsonSchemaCache.kt} (90%) rename formats/json/nativeMain/src/kotlinx/serialization/json/{schemaCache.kt => JsonSchemaCache.kt} (63%) diff --git a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt index f27af16be2..7ef3f40bac 100644 --- a/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt +++ b/formats/json/commonMain/src/kotlinx/serialization/json/Json.kt @@ -56,6 +56,11 @@ public sealed class Json( override val serializersModule: SerializersModule ) : StringFormat { + @Deprecated( + "Should not be accessed directly, use Json.schemaCache accessor instead", + ReplaceWith("schemaCache"), + DeprecationLevel.ERROR + ) internal val _schemaCache: DescriptorSchemaCache = DescriptorSchemaCache() /** @@ -292,7 +297,7 @@ private class JsonImpl(configuration: JsonConfiguration, module: SerializersModu } /** - * Emulate thread-locality of cache on Native + * This accessor should be used to workaround for freezing problems in Native, see Native source set */ internal expect val Json.schemaCache: DescriptorSchemaCache diff --git a/formats/json/jsMain/src/kotlinx/serialization/json/schemaCache.kt b/formats/json/jsMain/src/kotlinx/serialization/json/JsonSchemaCache.kt similarity index 90% rename from formats/json/jsMain/src/kotlinx/serialization/json/schemaCache.kt rename to formats/json/jsMain/src/kotlinx/serialization/json/JsonSchemaCache.kt index d303de8ebf..a0820ef4b7 100644 --- a/formats/json/jsMain/src/kotlinx/serialization/json/schemaCache.kt +++ b/formats/json/jsMain/src/kotlinx/serialization/json/JsonSchemaCache.kt @@ -6,4 +6,5 @@ package kotlinx.serialization.json import kotlinx.serialization.json.internal.* +@Suppress("DEPRECATION_ERROR") internal actual val Json.schemaCache: DescriptorSchemaCache get() = this._schemaCache diff --git a/formats/json/jvmMain/src/kotlinx/serialization/json/schemaCache.kt b/formats/json/jvmMain/src/kotlinx/serialization/json/JsonSchemaCache.kt similarity index 90% rename from formats/json/jvmMain/src/kotlinx/serialization/json/schemaCache.kt rename to formats/json/jvmMain/src/kotlinx/serialization/json/JsonSchemaCache.kt index d303de8ebf..a0820ef4b7 100644 --- a/formats/json/jvmMain/src/kotlinx/serialization/json/schemaCache.kt +++ b/formats/json/jvmMain/src/kotlinx/serialization/json/JsonSchemaCache.kt @@ -6,4 +6,5 @@ package kotlinx.serialization.json import kotlinx.serialization.json.internal.* +@Suppress("DEPRECATION_ERROR") internal actual val Json.schemaCache: DescriptorSchemaCache get() = this._schemaCache diff --git a/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt b/formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt similarity index 63% rename from formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt rename to formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt index f61580de27..879952bee4 100644 --- a/formats/json/nativeMain/src/kotlinx/serialization/json/schemaCache.kt +++ b/formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt @@ -8,11 +8,22 @@ import kotlinx.serialization.json.internal.* import kotlin.native.ref.* import kotlin.random.* +/** + * This maps emulate thread-locality of DescriptorSchemaCache for Native. + * + * Custom JSON instances are considered thread-safety (in JVM) and can be frozen and transferred to different workers (in Native). + * Therefore, DescriptorSchemaCache should be either a concurrent freeze-aware map or thread local. + * Each JSON instance have it's own schemaCache, and it's impossible to use @ThreadLocal on non-global vals. + * Thus we make @ThreadLocal this special map: it provides schemaCache for a particular Json instance + * and should be used instead of a member `_schemaCache` on Native. + * + * To avoid memory leaks (when Json instance is no longer in use), WeakReference is used with periodical self-cleaning. + */ @ThreadLocal private val jsonToCache: MutableMap = mutableMapOf() /** - * Because WeakReference itself does not have equals/hashCode + * Because WeakReference itself does not have proper equals/hashCode */ private class WeakJson(json: Json) { private val ref = WeakReference(json) @@ -28,7 +39,6 @@ private class WeakJson(json: Json) { } override fun hashCode(): Int = initialHashCode - } /** @@ -47,7 +57,7 @@ private fun cleanUpWeakMap() { } /** - * Emulate thread locality of cache on Native + * Accessor for DescriptorSchemaCache */ internal actual val Json.schemaCache: DescriptorSchemaCache get() = jsonToCache.getOrPut(WeakJson(this)) { DescriptorSchemaCache() }.also { cleanUpWeakMap() } From 53226571b3d6873b74b490db1a07a8f09c0fbc65 Mon Sep 17 00:00:00 2001 From: Leonid Startsev Date: Thu, 20 May 2021 16:37:06 +0300 Subject: [PATCH 4/4] Update formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt --- .../src/kotlinx/serialization/json/JsonSchemaCache.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt b/formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt index 879952bee4..16e223a0b0 100644 --- a/formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt +++ b/formats/json/nativeMain/src/kotlinx/serialization/json/JsonSchemaCache.kt @@ -11,7 +11,7 @@ import kotlin.random.* /** * This maps emulate thread-locality of DescriptorSchemaCache for Native. * - * Custom JSON instances are considered thread-safety (in JVM) and can be frozen and transferred to different workers (in Native). + * Custom JSON instances are considered thread-safe (in JVM) and can be frozen and transferred to different workers (in Native). * Therefore, DescriptorSchemaCache should be either a concurrent freeze-aware map or thread local. * Each JSON instance have it's own schemaCache, and it's impossible to use @ThreadLocal on non-global vals. * Thus we make @ThreadLocal this special map: it provides schemaCache for a particular Json instance