Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thread safety broken in 1.2.0 (Kotlin 1.5) #1455

Closed
bsimon36 opened this issue May 6, 2021 · 12 comments
Closed

Thread safety broken in 1.2.0 (Kotlin 1.5) #1455

bsimon36 opened this issue May 6, 2021 · 12 comments
Assignees

Comments

@bsimon36
Copy link
Contributor

bsimon36 commented May 6, 2021

Describe the bug
Thread safety seems to be broken in the newest version, causing the outputs of Json.encodeToString to get garbled with each other.

To Reproduce
for (i in 1..1000) {
thread {
val obj = (1..10000).map { Math.random() }
if (obj != Json.decodeFromString<List>(Json.encodeToString(obj))) {
println("Oh no!")
}
}
}

Expected behavior
Should run fine, instead prints out lots of errors from invalid json and also some "Oh no!"s

Environment

  • Kotlin version: [e.g. 1.5.0]
  • Library version: [e.g. 1.2.0]
  • Kotlin platforms: JVM
  • Gradle version: 6
  • IDE version (if bug is related to the IDE) [e.g. IntellijIDEA 2019.1, Android Studio 3.4]
  • Other relevant context [e.g. OS version, JRE version, ... ]
@bsimon36
Copy link
Contributor Author

bsimon36 commented May 6, 2021

sorry, formatted repro code:

for (i in 1..1000) {
  thread {
    val obj = (1..10000).map { Math.random() }
    if (obj != Json.decodeFromString<List<Double>>(Json.encodeToString(obj))) {
      println("Oh no!")
    }
  }
}

@RomanTruba
Copy link

Try to compare doubles with delta

@bsimon36
Copy link
Contributor Author

bsimon36 commented May 6, 2021

That's not the issue. The errors are mostly json that is invalid and fails to be parsed.

Exception in thread "Thread-762" kotlinx.serialization.json.internal.JsonDecodingException: Unexpected JSON token at offset 11400: Failed to parse type 'double' for input '0.0.21628340242132538'
JSON input: .....11891752,0.0.21628340242132538,0.1068633933226506,0.47438333.....
        at kotlinx.serialization.json.internal.JsonExceptionsKt.JsonDecodingException(JsonExceptions.kt:24)
        at kotlinx.serialization.json.internal.JsonExceptionsKt.JsonDecodingException(JsonExceptions.kt:32)
        at kotlinx.serialization.json.internal.JsonLexer.fail(JsonLexer.kt:477)
        at kotlinx.serialization.json.internal.JsonLexer.fail$default(JsonLexer.kt:476)
        at kotlinx.serialization.json.internal.StreamingJsonDecoder.decodeDouble(StreamingJsonDecoder.kt:310)
        at kotlinx.serialization.internal.DoubleSerializer.deserialize(Primitives.kt:126)
        at kotlinx.serialization.internal.DoubleSerializer.deserialize(Primitives.kt:122)
        at kotlinx.serialization.json.internal.PolymorphicKt.decodeSerializableValuePolymorphic(Polymorphic.kt:63)
        at kotlinx.serialization.json.internal.StreamingJsonDecoder.decodeSerializableValue(StreamingJsonDecoder.kt:32)
        at kotlinx.serialization.encoding.AbstractDecoder.decodeSerializableValue(AbstractDecoder.kt:43)
        at kotlinx.serialization.encoding.AbstractDecoder.decodeSerializableElement(AbstractDecoder.kt:70)
        at kotlinx.serialization.encoding.CompositeDecoder$DefaultImpls.decodeSerializableElement$default(Decoding.kt:535)
        at kotlinx.serialization.internal.ListLikeSerializer.readElement(CollectionSerializers.kt:80)
        at kotlinx.serialization.internal.AbstractCollectionSerializer.readElement$default(CollectionSerializers.kt:51)
        at kotlinx.serialization.internal.AbstractCollectionSerializer.merge(CollectionSerializers.kt:36)
        at kotlinx.serialization.internal.AbstractCollectionSerializer.deserialize(CollectionSerializers.kt:43)
        at kotlinx.serialization.json.internal.PolymorphicKt.decodeSerializableValuePolymorphic(Polymorphic.kt:63)
        at kotlinx.serialization.json.internal.StreamingJsonDecoder.decodeSerializableValue(StreamingJsonDecoder.kt:32)
        at kotlinx.serialization.json.Json.decodeFromString(Json.kt:95)
        at com.downdogapp.tasks.Scratch$main$1.invoke(Scratch.kt:31)

@RomanTruba
Copy link

@bsimon36 makes sense, sorry for missing that point

@bsimon36
Copy link
Contributor Author

bsimon36 commented May 7, 2021

I should note that we caught this in production. The problem only occurs with enough multi-threading so it is likely that people will test the new version, think it's working fine, but then have it break (in possibly hard-to-detect ways) when deployed to an actual server serving many requests simultaneously.

@jurmous
Copy link

jurmous commented May 10, 2021

I have seen this too on my local machine when testing with latest Ktor server api and latest Kotlin. Every few refreshes I receive a garbled JSON from one of the calls. Downgrading to earlier version of this framework fixes the issue and makes the output consistent again..

@necatisozer
Copy link

We also faced that issue with 1.2.0 version in production. Fortunately we were able to reproduce it.

import kotlinx.coroutines.*
import kotlinx.serialization.Serializable
import kotlinx.serialization.decodeFromString
import kotlinx.serialization.encodeToString
import kotlinx.serialization.json.Json

@Serializable
data class Foo(val bar: Bar, val baz: String? = null)

@Serializable
data class Bar(val string: String, val integer: Int, val double: Double, val boolean: Boolean)

fun main() = runBlocking {
    val foo1 = Foo(Bar("string1", 1, 1.0, true), "baz1")
    val foo2 = Foo(Bar("string2", 2, 2.0, false))

    repeat(1_000) {
        CoroutineScope(Dispatchers.Default).launch {
            Json.decodeFromString<Foo>(Json.encodeToString(foo1))
        }

        CoroutineScope(Dispatchers.Default).launch {
            Json.decodeFromString<Foo>(Json.encodeToString(foo2))
        }

        delay(1)
    }
}

As you realize, encoded string values are garbled with each other.


Exception in thread "DefaultDispatcher-worker-3" kotlinx.serialization.json.internal.JsonDecodingException: Unexpected JSON token at offset 70: Expected EOF, but had b instead
JSON input: {"bar":{"string":"string2","integer":2,"double":2.0,"boolean":false}}baz":"baz1"}
	at kotlinx.serialization.json.internal.JsonExceptionsKt.JsonDecodingException(JsonExceptions.kt:24)
	at kotlinx.serialization.json.internal.JsonExceptionsKt.JsonDecodingException(JsonExceptions.kt:32)
	at kotlinx.serialization.json.internal.JsonLexer.fail(JsonLexer.kt:477)
	at kotlinx.serialization.json.internal.JsonLexer.fail$default(JsonLexer.kt:476)
	at kotlinx.serialization.json.internal.JsonLexer.expectEof(JsonLexer.kt:130)
	at kotlinx.serialization.json.Json.decodeFromString(Json.kt:96)
	at com.simgesengun.thirdapplication.MainKt$main$1$1$1.invokeSuspend(Main.kt:34)
	at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33)
	at kotlinx.coroutines.DispatchedTask.run(DispatchedTask.kt:106)
	at kotlinx.coroutines.scheduling.CoroutineScheduler.runSafely(CoroutineScheduler.kt:571)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.executeTask(CoroutineScheduler.kt:750)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.runWorker(CoroutineScheduler.kt:678)
	at kotlinx.coroutines.scheduling.CoroutineScheduler$Worker.run(CoroutineScheduler.kt:665)

When we revert back to 1.1.0 version, we don't face that issue.

@valeriyo
Copy link

Oops. I opened an issue in https://youtrack.jetbrains.com/issue/KT-46623 about this problem.

@valeriyo
Copy link

@qwwdfsad and @sandwwraith, Please consider un-releasing 1.2.0. This is a pretty serious issue that may sneak to production undetected, and break a lot of stuff. In my case, it cause a lot of errors (local and with server) as well as exposed a battery draining bug (#1474) -- Seems very bad to let people upgrade to 1.2.0 knowing that these are likely to happen. Pull it until there is a patch with a fix.

@ScottPeterJohnson
Copy link

Can confirm that I've seen this issue and it appears to be quite serious.

@qwwdfsad
Copy link
Contributor

We'll release 1.2.1 around tomorrow with all the fixes. Unfortunately, we cannot revoke 1.2.0 from MC, so the best we can do here is a clear warning during all the announcements

@valeriyo
Copy link

@qwwdfsad perhaps, the "What's new in Kotlin 1.5" https://kotlinlang.org/docs/whatsnew15.html#serialization-1-2-0 could be updated to mention 1.2.1 instead of 1.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants