Skip to content

Commit

Permalink
Ensure serialization is usable from K/N background thread
Browse files Browse the repository at this point in the history
* fix concurrency issue when accessing C2TC

This bytearray was mutating itself during the init phase. By moving it to an object, the init {} deals with the initialization and makes it thread safe. The same logic was used for ESCAPE_2_CHAR.

* Serialization meets K/N background threads

    * Run all tests from both main and bg threads
    * Update Gradle distribution from bin to all
    * Update JsonReader, get rid of companions and late-initialization to make it K/N friendly and produce less code
    * Update tests to be bg-friendly

Co-authored-by: Mohamed Zenadi <[email protected]>
  • Loading branch information
qwwdfsad and zeapo authored Jan 22, 2021
1 parent 2d96a71 commit 07f730a
Show file tree
Hide file tree
Showing 16 changed files with 80 additions and 54 deletions.
3 changes: 2 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ buildscript {

// To make it visible for compiler-version.gradle
ext.compilerVersion = org.jetbrains.kotlin.config.KotlinCompilerVersion.VERSION
ext.nativeDebugBuild = org.jetbrains.kotlin.gradle.plugin.mpp.NativeBuildType.DEBUG
apply plugin: 'binary-compatibility-validator'

apiValidation {
Expand Down Expand Up @@ -184,4 +185,4 @@ if (jvm_ir_enabled) {
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package kotlinx.serialization

import kotlinx.serialization.modules.*
import kotlin.native.concurrent.*

@Serializable
open class PolyBase(val id: Int) {
Expand All @@ -29,6 +30,7 @@ open class PolyBase(val id: Int) {
@Serializable
data class PolyDerived(val s: String) : PolyBase(1)

@SharedImmutable
val BaseAndDerivedModule = SerializersModule {
polymorphic(PolyBase::class, PolyBase.serializer()) {
subclass(PolyDerived.serializer())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,7 @@
package kotlinx.serialization.test

import kotlinx.serialization.test.Platform
import kotlin.native.concurrent.SharedImmutable

@SharedImmutable
public actual val currentPlatform: Platform = Platform.NATIVE
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
package kotlinx.serialization

import kotlinx.serialization.modules.*
import kotlin.native.concurrent.*

@Serializable
abstract class SimpleAbstract
Expand All @@ -18,6 +19,7 @@ data class SimpleStringInheritor(val s: String, val i: Int) : SimpleAbstract()
@Serializable
data class PolyBox(@Polymorphic val boxed: SimpleAbstract)

@SharedImmutable
val SimplePolymorphicModule = SerializersModule {
polymorphic(SimpleAbstract::class) {
subclass(SimpleIntInheritor.serializer())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@

package kotlinx.serialization.json.internal

import kotlinx.serialization.json.internal.EscapeCharMappings.ESCAPE_2_CHAR
import kotlinx.serialization.json.internal.CharMappings.C2TC
import kotlinx.serialization.json.internal.CharMappings.ESCAPE_2_CHAR
import kotlin.jvm.*
import kotlin.native.concurrent.*

internal const val lenientHint = "Use 'isLenient = true' in 'Json {}` builder to accept non-compliant JSON."
internal const val coerceInputValuesHint = "Use 'coerceInputValues = true' in 'Json {}` builder to coerce nulls to default values."
Expand Down Expand Up @@ -51,32 +51,20 @@ private const val CTC_MAX = 0x7e
// mapping from escape chars real chars
private const val ESC2C_MAX = 0x75

@SharedImmutable
internal val C2TC = ByteArray(CTC_MAX).apply {
for (i in 0..0x20) {
initC2TC(i, TC_INVALID)
}

initC2TC(0x09, TC_WS)
initC2TC(0x0a, TC_WS)
initC2TC(0x0d, TC_WS)
initC2TC(0x20, TC_WS)
initC2TC(COMMA, TC_COMMA)
initC2TC(COLON, TC_COLON)
initC2TC(BEGIN_OBJ, TC_BEGIN_OBJ)
initC2TC(END_OBJ, TC_END_OBJ)
initC2TC(BEGIN_LIST, TC_BEGIN_LIST)
initC2TC(END_LIST, TC_END_LIST)
initC2TC(STRING, TC_STRING)
initC2TC(STRING_ESC, TC_STRING_ESC)
}
// object instead of @SharedImmutable because there is mutual initialization in [initC2ESC] and [initC2TC]
internal object CharMappings {
@JvmField
val ESCAPE_2_CHAR = CharArray(ESC2C_MAX)

// object instead of @SharedImmutable because there is mutual initialization in [initC2ESC]
internal object EscapeCharMappings {
@JvmField
public val ESCAPE_2_CHAR = CharArray(ESC2C_MAX)
val C2TC = ByteArray(CTC_MAX)

init {
initEscape()
initCharToToken()
}

private fun initEscape() {
for (i in 0x00..0x1f) {
initC2ESC(i, UNICODE_ESC)
}
Expand All @@ -91,19 +79,36 @@ internal object EscapeCharMappings {
initC2ESC(STRING_ESC, STRING_ESC)
}

private fun initCharToToken() {
for (i in 0..0x20) {
initC2TC(i, TC_INVALID)
}

initC2TC(0x09, TC_WS)
initC2TC(0x0a, TC_WS)
initC2TC(0x0d, TC_WS)
initC2TC(0x20, TC_WS)
initC2TC(COMMA, TC_COMMA)
initC2TC(COLON, TC_COLON)
initC2TC(BEGIN_OBJ, TC_BEGIN_OBJ)
initC2TC(END_OBJ, TC_END_OBJ)
initC2TC(BEGIN_LIST, TC_BEGIN_LIST)
initC2TC(END_LIST, TC_END_LIST)
initC2TC(STRING, TC_STRING)
initC2TC(STRING_ESC, TC_STRING_ESC)
}

private fun initC2ESC(c: Int, esc: Char) {
if (esc != UNICODE_ESC) ESCAPE_2_CHAR[esc.toInt()] = c.toChar()
}

private fun initC2ESC(c: Char, esc: Char) = initC2ESC(c.toInt(), esc)
}

private fun ByteArray.initC2TC(c: Int, cl: Byte) {
this[c] = cl
}
private fun initC2TC(c: Int, cl: Byte) {
C2TC[c] = cl
}

private fun ByteArray.initC2TC(c: Char, cl: Byte) {
initC2TC(c.toInt(), cl)
private fun initC2TC(c: Char, cl: Byte) = initC2TC(c.toInt(), cl)
}

internal fun charToTokenClass(c: Char) = if (c.toInt() < CTC_MAX) C2TC[c.toInt()] else TC_OTHER
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package kotlinx.serialization

import kotlinx.serialization.json.*
import kotlinx.serialization.modules.*
import kotlin.native.concurrent.*

@Serializable
open class PolyBase(val id: Int) {
Expand Down Expand Up @@ -34,6 +35,7 @@ data class PolyDefault(val json: JsonElement) : PolyBase(-1)
@Serializable
data class PolyDerived(val s: String) : PolyBase(1)

@SharedImmutable
val BaseAndDerivedModule = SerializersModule {
polymorphic(PolyBase::class, PolyBase.serializer()) {
subclass(PolyDerived.serializer())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package kotlinx.serialization.json.polymorphic
import kotlinx.serialization.*
import kotlinx.serialization.json.*
import kotlinx.serialization.modules.*
import kotlin.native.concurrent.*

@Serializable
internal open class InnerBase
Expand Down Expand Up @@ -37,7 +38,7 @@ internal data class OuterBox(@Polymorphic val outerBase: OuterBase, @Polymorphic
@Serializable
internal data class OuterNullableBox(@Polymorphic val outerBase: OuterBase?, @Polymorphic val innerBase: InnerBase?)


@SharedImmutable
internal val polymorphicTestModule = SerializersModule {
polymorphic(InnerBase::class) {
subclass(InnerImpl.serializer())
Expand All @@ -50,11 +51,13 @@ internal val polymorphicTestModule = SerializersModule {
}
}

@SharedImmutable
internal val polymorphicJson = Json {
serializersModule = polymorphicTestModule
encodeDefaults = true
}

@SharedImmutable
internal val polymorphicRelaxedJson = Json {
isLenient = true
serializersModule = polymorphicTestModule
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,8 @@

package kotlinx.serialization.test

import kotlin.native.concurrent.SharedImmutable


@SharedImmutable
public actual val currentPlatform: Platform = Platform.NATIVE
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package kotlinx.serialization

import kotlinx.serialization.modules.*
import kotlinx.serialization.protobuf.*
import kotlin.native.concurrent.*

@Serializable
open class PolyBase(@ProtoNumber(1) val id: Int) {
Expand Down Expand Up @@ -45,6 +46,7 @@ data class SimpleStringInheritor(val s: String, val i: Int) : SimpleAbstract()
@Serializable
data class PolyBox(@Polymorphic val boxed: SimpleAbstract)

@SharedImmutable
val SimplePolymorphicModule = SerializersModule {
polymorphic(SimpleAbstract::class) {
subclass(SimpleIntInheritor.serializer())
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
group=org.jetbrains.kotlinx
version=1.1.0-SNAPSHOT

kotlin.version=1.4.30-M1
kotlin.version=1.4.30-RC

# This version take precedence if 'bootstrap' property passed to project
kotlin.version.snapshot=1.4.255-SNAPSHOT
Expand Down
16 changes: 16 additions & 0 deletions gradle/configure-source-sets.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ kotlin {
}
}
}

js {
nodejs {}
configure([compilations.main, compilations.test]) {
Expand Down Expand Up @@ -87,4 +88,19 @@ kotlin {
sourceSets.matching({ it.name.contains("Main") }).all { srcSet ->
project.ext.set("kotlin.mpp.freeCompilerArgsForSourceSet.${srcSet.name}", "-Xexplicit-api=warning")
}

def targetsWithoutTestRunners = ["linuxArm32Hfp", "linuxArm64", "mingwX86"]
configure(targets) {
// Configure additional binaries to run tests in the background
if (["macos", "linux", "mingw"].any { name.startsWith(it) && !targetsWithoutTestRunners.contains(name) }) {
binaries {
test("background", [nativeDebugBuild]) {
freeCompilerArgs += ["-trw"]
}
}
testRuns {
background { setExecutionSourceFrom(binaries.backgroundDebugTest) }
}
}
}
}
1 change: 0 additions & 1 deletion gradle/native-targets.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ kotlin {

targets {
if (project.ext.nativeState == NativeState.DISABLED) return

if (project.ext.singleTargetMode) {
fromPreset(project.ext.ideaPreset, 'native')
} else {
Expand Down
Binary file modified gradle/wrapper/gradle-wrapper.jar
Binary file not shown.
6 changes: 5 additions & 1 deletion gradle/wrapper/gradle-wrapper.properties
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#
# Copyright 2017-2021 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license.
#

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
distributionUrl=https\://services.gradle.org/distributions/gradle-6.7.1-bin.zip
distributionUrl=https\://services.gradle.org/distributions/gradle-6.7.1-all.zip
zipStoreBase=GRADLE_USER_HOME
zipStorePath=wrapper/dists
2 changes: 1 addition & 1 deletion gradlew
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ fi
if [ "$cygwin" = "true" -o "$msys" = "true" ] ; then
APP_HOME=`cygpath --path --mixed "$APP_HOME"`
CLASSPATH=`cygpath --path --mixed "$CLASSPATH"`

JAVACMD=`cygpath --unix "$JAVACMD"`

# We build the pattern for arguments to be converted via cygpath
Expand Down
21 changes: 3 additions & 18 deletions gradlew.bat
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ if defined JAVA_HOME goto findJavaFromJavaHome

set JAVA_EXE=java.exe
%JAVA_EXE% -version >NUL 2>&1
if "%ERRORLEVEL%" == "0" goto init
if "%ERRORLEVEL%" == "0" goto execute

echo.
echo ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
Expand All @@ -54,7 +54,7 @@ goto fail
set JAVA_HOME=%JAVA_HOME:"=%
set JAVA_EXE=%JAVA_HOME%/bin/java.exe

if exist "%JAVA_EXE%" goto init
if exist "%JAVA_EXE%" goto execute

echo.
echo ERROR: JAVA_HOME is set to an invalid directory: %JAVA_HOME%
Expand All @@ -64,29 +64,14 @@ echo location of your Java installation.

goto fail

:init
@rem Get command-line arguments, handling Windows variants

if not "%OS%" == "Windows_NT" goto win9xME_args

:win9xME_args
@rem Slurp the command line arguments.
set CMD_LINE_ARGS=
set _SKIP=2

:win9xME_args_slurp
if "x%~1" == "x" goto execute

set CMD_LINE_ARGS=%*

:execute
@rem Setup the command line

set CLASSPATH=%APP_HOME%\gradle\wrapper\gradle-wrapper.jar


@rem Execute Gradle
"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %CMD_LINE_ARGS%
"%JAVA_EXE%" %DEFAULT_JVM_OPTS% %JAVA_OPTS% %GRADLE_OPTS% "-Dorg.gradle.appname=%APP_BASE_NAME%" -classpath "%CLASSPATH%" org.gradle.wrapper.GradleWrapperMain %*

:end
@rem End local scope for the variables with windows NT shell
Expand Down

0 comments on commit 07f730a

Please sign in to comment.