Hi, let me borrow your brain for a second here, I ...

# test

[Marco Garofalo]

Hi, let me borrow your brain for a second here, I am trying to prove the following code is not thread-safe by writing a test for it, make it fail (consistently) and fix the issue, but I am really struggling to make it fail LOL.

Here’s the code:

package com.example.gameplay

class RotatingSecrets(private val secrets: List<String>) : Secrets {
    private var position = 0

    override fun next(): String {
        return secrets[position++ % secrets.size]
    }
}

This is what I have tried so far:

package com.example.gameplay

import io.kotest.matchers.equals.shouldBeEqual
import kotlinx.coroutines.CoroutineStart.LAZY
import kotlinx.coroutines.async
import kotlinx.coroutines.awaitAll
import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Test

class RotatingSecretsTests {

    @Test
    fun `is thread safe`() = runTest {
        val concurrentAccesses = 100
        val secrets = (1..concurrentAccesses).map { "secret-$it" }
        val rotatingSecrets = RotatingSecrets(secrets)

        val jobs = secrets.map {
            async (start = LAZY) {
                rotatingSecrets.next()
            }
        }

        val seenSecrets = jobs.awaitAll()
        seenSecrets.sorted() shouldBeEqual secrets.sorted()
    }
}

I have also tried increasing the concurrency up to 10_000, but no luck.

Any clever idea?

[Peter Farlow]

Why did you sort seenSecrets and secrets before comparing them? What happens if you compare them unsorted?

[Marco Garofalo]

I am sorting them just because I am interested in the content, not the order, so if for some reason the coroutines result end up being all over the place I want the test to pass.

[Peter Farlow]

Can define exactly what ‘thread-safe’ means in the context of the use case for your RotatingSecrets class?

[Marco Garofalo]

Well that class uses an increment operation, which is by definition not thread-safe, since is like 2 instructions in one, so depending when the threads access the data they could end up reading stale data.

var x = 0
val y = x++

=>

val y = x
x = x + 1

That said, I have the suspicion that Kotlin might compile that class in someway that prevents the race condition, since is just a very simple code.

[Peter Farlow]

I understand that, I’m just trying to understand how you think that will apply to your code. What would an incorrect output look like?

[Peter Farlow]

you said you aren’t interested in order, so I don’t know what you want the failure case to look like

[Peter Farlow]

I don’t see how the content could change in this class

[Marco Garofalo]

Well I am expecting gaps in the result, i.e. “secret-33” repeated twice, meaning that it will miss “secret-100"

[Peter Farlow]

got it. So did you try using a multi-threaded coroutine dispatcher?

[Marco Garofalo]

As with any non thread-safe operation, I am expecting the position variable to have every now and then the wrong value, creating gaps in the result

[Marco Garofalo]

Yeah right now I am using this:

@Test
    fun `is thread safe`() = runTest {
        val concurrentAccesses = 100
        val secrets = (1..concurrentAccesses).map { "secret-$it" }
        val rotatingSecrets = RotatingSecrets(secrets)
        val threads = newFixedThreadPoolContext(10, "rotating-secrets-test")

        val jobs = secrets.map {
            async (start = LAZY, context = threads) {
                delay((100..200L).random())
                println("[${Thread.currentThread().name}] running")
                rotatingSecrets.next()
            }
        }

        val seenSecrets = jobs.awaitAll()
        seenSecrets.sorted() shouldBeEqual secrets.sorted()
    }

[Marco Garofalo]

I have also added some arbitrary delay, but still works fine 🤣

[ephemient]

runTest is not helping you here at all - its goal is to make tests fast and repeatable. might as well dump it

[ephemient]

have you considered https://github.com/JetBrains/lincheck?

[Marco Garofalo]

Thanks @ephemient that worked like a charm!

class RotatingSecretsTests {

    private val rotatingSecrets = RotatingSecrets(listOf("first", "second"))

    @Operation
    fun nextSecret() = rotatingSecrets.next()

    @Test
    fun `is thread safe`() = StressOptions()
        .threads(2)
        .iterations(10)
        .check(this::class)
}