ansman/kotshi

Possible race condition in KSP factory generation

Closed this issue · 12 comments

We've just switched from Kapt to KSP for JSON generation of the adapters. The build is a multi-module Gradle project.

The KAPT generators are rock solid, but it seems like when we run through the KSP generation, fairly regularly the individual Kotlin adapters are generated, but the equivalent module JsonFactory implementations are occasionally empty, and generate like below:

@OptIn(InternalKotshiApi::class)
internal object KotshiSandboxJsonAdapterFactory : JsonAdapter.Factory {
  public override fun create(
    type: Type,
    annotations: Set<Annotation>,
    moshi: Moshi,
  ): JsonAdapter<*>? = null
}

This happens on multiple machines, and we can't see a pattern. Our one guess is that there is possibly a race condition going on and that the Factory is generated before the individual adapters... does that seem possible?

Interesting, I wonder if this is because of incremental compilation. Does it happen on a clean build too?

$ ./gradlew --rerun-tasks --no-build-cache <assemble-task>

I suppose it could be - we haven't seen it on our build servers, and I have just run the build a couple of times using the above command and was all good. Will keep monitoring it and report back if it doesn't work (although obvs the incremental compilation is v useful :)

BTW, we're on:

MacOS
Java 11/17
Gradle 7.6
Kotlin 1.8.0
Kotshi 2.9.1

Must be something at play with Kotlin 1.8.0, seems when we switch to Kotlin 1.7.22 it works as expected.

I'm not sure if this is related, but we also had some problems when there were multiple "source" kotlin classes in a single Kotlin file, causing the actual adapter to not get generated ... moving these out into their own files seemed to fix that problem. (same environment as above 😄 )

Interesting, I wonder if this is because of incremental compilation. Does it happen on a clean build too?

$ ./gradlew --rerun-tasks --no-build-cache <assemble-task>

More on this - it was a workaround that worked on the command line, but IntelliJ was still a problem.

IntelliJ version for completeness

IntelliJ IDEA 2022.3 (Ultimate Edition)
Build #IU-223.7571.182, built on November 29, 2022
Runtime version: 17.0.5+1-b653.14 x86_64
VM: OpenJDK 64-Bit Server VM by JetBrains s.r.o.
macOS 12.6
GC: G1 Young Generation, G1 Old Generation
Memory: 4096M
Cores: 12
Kotlin: 223-1.7.20-release-201-IJ7571.182

I added a test for incremental kompilation with KSP in #198 though it's unclear if the test library actually supports it.

I'll see if I can actually test incremental for real later this week.

Just released 2.10.0 with Kotlin 1.8 support, let me know if that one still has the issues.

Thanks so much for the quick turnaround..

Well the good news is that we get further in the process and the adapters generate (and the factories as well). The slightly less great news is that I've uncovered a really subtle bug in the KSP generator. If you have a class where:

  • the field name is the same as its class
    AND
  • the field is nullable
    AND
  • the field has a default value

... the KSP generated class does not compile. Try this and you can see:

@JsonSerializable
data class MyClazz(
    val Int: Int? = null
)

I can raise another issue if you would like to track that differently @ansman ? 😄

(and yes - that wasn't easy to track down!)

Nice catch, I'll see if I can fix that one tonight. Should be easy since we have a repro.

And yes, if you could open a separate issue so we can keep this for the factory issues that would be great. Otherwise I can open one tonight

Here you go (#199).

We can probably close this one since it seems to be fixed.

This is fixed now! :)