Dagger compiler breaks on classloader issue using bazel rules_kotlin/kt_ksp_plugin and jvm_rules_external

[cgruber]

I am working on a project (dagger-grpc) which provides a different approach to dagger binding, with a focus on direct examples of integration with microservice containers (armeria in particular). I am building out the examples, and running into a difficulty wherein Dagger, when run as a KSP processor, blows up with an IllegalAccessException from a module violation, in attempting to reference a class in com.google.common.graph from another which has already been loaded in the outer container. (see trace below).

I referenced this in this comment

Environment:

• bazel_dep(name = "rules_kotlin", version = "2.1.0", repo_name = "io_bazel_rules_kotlin")
• Bazel 8.1.1
• OS: Mac/OS 15.3.1
• Dagger 2.55 (via maven_install / rules_jvm_external)

All bazel-isms are set up locally for the project, and use maven-downloaded deps. Notably, I set up a plugin declaration for dagger-compiler like so in my root BUILD.bazel file:

load("@io_bazel_rules_kotlin//kotlin:core.bzl", "kt_ksp_plugin")

kt_ksp_plugin(
    name = "dagger-compiler",
    processor_class = "dagger.internal.codegen.ComponentProcessor",
    visibility = ["//visibility:public"],
    deps = [
        "@maven//:com_google_dagger_dagger_compiler",
    ],
)

And then my target is like so:

kt_jvm_library(
    name = "lib",
    srcs = glob(["src/main/kotlin/**/*.kt"]),
    plugins = [
        "@dagger-grpc//io_grpc/compiler/ksp:plugin",
        "//:dagger-compiler",
    ],
    visibility = ["//visibility:public"],
    runtime_deps = [
        "@maven//:org_slf4j_slf4j_jdk14",
    ],
    deps = [
        "//proto:grpc",
        "@com_google_protobuf//java/core",
        "@dagger-grpc//api",
        "@dagger-grpc//third_party/dagger",  # Use your own dagger mapping in your repo
        "@dagger-grpc//util/armeria",
        "@maven//:com_google_guava_guava",
        "@maven//:com_linecorp_armeria_armeria_grpc",
        "@maven//:io_github_oshai_kotlin_logging_jvm",
        "@maven//:io_grpc_grpc_protobuf_lite",
    ],
)

Expected behavior

Normal generation of dagger factories and component implementations.

Actual Outcome

Stack trace:

exception: java.lang.IllegalAccessError: failed to access class com.google.common.graph.BaseGraph from class com.google.common.graph.Traverser (com.google.common.graph.BaseGraph is in unnamed module of loader org.jetbrains.kotlin.preloading.MemoryBasedClassLoader @50b494a6; com.google.common.graph.Traverser is in unnamed module of loader java.net.URLClassLoader @268e3a1d)
	at com.google.common.graph.Traverser.forTree(Traverser.java:182)
	at dagger.internal.codegen.validation.ValidationReport.<clinit>(ValidationReport.java:43)
	at dagger.internal.codegen.validation.InjectValidator$InternalValidator.validateUncached(InjectValidator.java:159)
	at dagger.internal.codegen.base.Util.reentrantComputeIfAbsent(Util.java:33)
	at dagger.internal.codegen.validation.InjectValidator$InternalValidator.validate(InjectValidator.java:155)
	at dagger.internal.codegen.validation.InjectValidator.validate(InjectValidator.java:113)
	at dagger.internal.codegen.validation.InjectBindingRegistryImpl.tryRegisterConstructor(InjectBindingRegistryImpl.java:257)
	at dagger.internal.codegen.validation.InjectBindingRegistryImpl.tryRegisterInjectConstructor(InjectBindingRegistryImpl.java:243)
	at dagger.internal.codegen.processingstep.InjectProcessingStep.process(InjectProcessingStep.java:73)
	at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.lambda$process$0(TypeCheckingProcessingStep.java:94)
	at com.google.common.collect.RegularImmutableMap.forEach(RegularImmutableMap.java:297)
	at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:72)
	at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:49)
	at dagger.spi.internal.shaded.androidx.room.compiler.processing.XProcessingStep.process(XProcessingStep.kt:57)
	at dagger.spi.internal.shaded.androidx.room.compiler.processing.CommonProcessorDelegate.processRound(XBasicAnnotationProcessor.kt:134)
	at dagger.spi.internal.shaded.androidx.room.compiler.processing.ksp.KspBasicAnnotationProcessor.process(KspBasicAnnotationProcessor.kt:62)
...

Note: The presence of the other ksp plugin is immaterial - the issue reproduces irrespective of other plugins being present.

Reproduction Steps

I built a repro PR at geekinasuit/dagger-grpc#8

Analysis

The KSP environment provided by rules_kotlin is insufficiently isolated in its classloading. Something inside the ksp infrastructure itself uses parts of com.google.common.graph, but does not load all of it. Some provisional attempts to do a low-hanging-fruit fix on that end have failed (e.g. bazelbuild/rules_kotlin#1284).

Regardless of isolation on the containing end, dagger currently seems to be leaning into com.google.common.graph from within its own validation code (ValidationReport.java:43 based on the stack trace). This suggests com.google.common.graph is not "shaded" in the compiler maven artifact, leaving it exposed to collision with other usages of Guava in the containing environment. One fix of this, for Dagger, at least, would be to ensure that com.google.common.graph is package-rewritten to avoid the collision and class-loading module conflict. That was always best-practice for annotation processors anyway, back when I was involved with the project, but it may have changed since.

[bcorso]

I haven't taken a deep look into this (and I've never tried to setup KSP in Bazel before), but just a few things I've noticed:

I set up a plugin declaration for dagger-compiler like so in my root BUILD.bazel file:

kt_ksp_plugin(
   name = "dagger-compiler",
   processor_class = "dagger.internal.codegen.ComponentProcessor",
   visibility = ["//visibility:public"],
   deps = [
       "@maven//:com_google_dagger_dagger_compiler",
   ],
)

From the docs it seems like you probably need to use Dagger's KSP processor provider, dagger.internal.codegen.KspComponentProcessor.Provider?

Also, note that the kotlin_rules documentation states the following (https://github.com/bazelbuild/rules_kotlin?tab=readme-ov-file#ksp-kotlin-symbol-processing):

A few things to note:

• As of now rules_kotlin only supports KSP plugins that generate Kotlin code.
• KSP is not thread safe and will likely fail if you are using it in a build that has multiplex workers enabled.

Note that Dagger doesn't generate Kotlin source (though it's something we're actively working towards), so you might end up hitting an issue due to that even if you get the other issues fixed.

[cgruber]

@bcorso - I'm looking at a solution involving shading, and I'm having some trouble sorting through the release infra. The tooling looks like it assumes that if you're going to shade something, it's a whole dependency. There's no room for shading only some packages, from what I can tell. I may be wrong though. In my testing, locally, it was hard to work the rules to get the outcome. Any chance I could get some help navigating the shading system?

[cgruber]

You might be right abotu the provider processor. Let me check on that. But I think the bazel rules don't use the ServiceLoader mechanism but directly tees up the processor directly. The Provider is there purely for ServiceLoader, which only works within build systems that are built for it. Bazel's mechanism is more explicit.

[cgruber]

As to thread-safe - have added a rule to make sure only one ksp worker is running at a time. I already checked that issue. I hadn't cotton'ed on to the "generates java" part, that could be an issue. hrm. I'm (ostensibly, somewhat errant) a maintainer on rules_kotlin, so I'll be following up on both the isolation issue, and the generate-java issue.

[cgruber]

I may just pause my own effort also on the kt side, and just do the java/apt variant, which I was going to do second. Dagger generating Java may make that the only viable option in the moment, due to the lack of support of ksp->dagger in the kotlin rules.

[cgruber]

As of now rules_kotlin only supports KSP plugins that generate Kotlin code.

Looks like as of bazelbuild/rules_kotlin#1139 that is no longer the case, and the docs are out of date.

[bcorso]

Looks like as of bazelbuild/rules_kotlin#1139 that is no longer the case, and the docs are out of date.

That's good to know.

You might be right abotu the provider processor. Let me check on that. But I think the bazel rules don't use the ServiceLoader mechanism but directly tees up the processor directly. The Provider is there purely for ServiceLoader, which only works within build systems that are built for it. Bazel's mechanism is more explicit.

Okay, yeah I was mostly just judging based on the rules_kotlin docs that you should pass in the provider. However, either way, you should be using the KspComponentProcessor rather than ComponentProcessor used in your original post. The latter is a Javac processor, not a KSP processor so I'm not sure how that could work with KSP.

I'm looking at a solution involving shading, and I'm having some trouble sorting through the release infra. The tooling looks like it assumes that if you're going to shade something, it's a whole dependency. There's no room for shading only some packages, from what I can tell. I may be wrong though. In my testing, locally, it was hard to work the rules to get the outcome. Any chance I could get some help navigating the shading system?

The shading is done here:

https://source.corp.google.com/piper///depot/google3/third_party/java_src/dagger/project/util/deploy-dagger.sh;l=10

We currently don't shade Guava from the processor artifacts. We can definitely consider shading Guava, though I'm a little hesitant before understanding why it's needed. Ideally I would think bazel would just rely on the maven.install resolution strategy to pull in the version (or greater) that Dagger requires. That said, we have had issues in the past with Guava, specifically with users pulling in both the jre and android versions of Guava onto the processor classpath. Due to that, we currently avoid using things like ImmutableList#toImmutableList() in the codebase since those methods aren't (or weren't) available in the android versions of Guava. Not sure if that's what is going on in this case.

[cgruber]

It's not a version conflict, I don't think. I think it's that the BaseGraph class is loaded by the KSP environment somewhere, in its own classloader, and the Traverser isn't, but then Traverser gets loaded in the Memory class loader by Dagger, from within the KSP environment, and it can't load from the parent because it ends up crossing java9 module boundaries. Or something like that. This definitely isn't an android-version issue, which would show as a missing method.

I looked at deploy-dagger.sh, but when I do it, for a half minute I had com.google.common.graph loaded into the -spi artifact, but not re-packaged for some reason. I tried something else that didn't work, and when I went back, it wasn't even doing that again. I'll give it another go.

[cgruber]

Also, it definitely is running from the ksp plugin. The more full classpath (I trimmed) is:

exception: java.lang.IllegalAccessError: failed to access class com.google.common.graph.BaseGraph from class com.google.common.graph.Traverser (com.google.common.graph.BaseGraph is in unnamed module of loader org.jetbrains.kotlin.preloading.MemoryBasedClassLoader @50b494a6; com.google.common.graph.Traverser is in unnamed module of loader java.net.URLClassLoader @247c0b0f)
	at com.google.common.graph.Traverser.forTree(Traverser.java:182)
	at dagger.internal.codegen.validation.ValidationReport.<clinit>(ValidationReport.java:43)
	at dagger.internal.codegen.validation.InjectValidator$InternalValidator.validateUncached(InjectValidator.java:159)
	at dagger.internal.codegen.base.Util.reentrantComputeIfAbsent(Util.java:33)
	at dagger.internal.codegen.validation.InjectValidator$InternalValidator.validate(InjectValidator.java:155)
	at dagger.internal.codegen.validation.InjectValidator.validate(InjectValidator.java:113)
	at dagger.internal.codegen.validation.InjectBindingRegistryImpl.tryRegisterConstructor(InjectBindingRegistryImpl.java:257)
	at dagger.internal.codegen.validation.InjectBindingRegistryImpl.tryRegisterInjectConstructor(InjectBindingRegistryImpl.java:243)
	at dagger.internal.codegen.processingstep.InjectProcessingStep.process(InjectProcessingStep.java:73)
	at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.lambda$process$0(TypeCheckingProcessingStep.java:94)
	at com.google.common.collect.RegularImmutableMap.forEach(RegularImmutableMap.java:297)
	at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:72)
	at dagger.internal.codegen.processingstep.TypeCheckingProcessingStep.process(TypeCheckingProcessingStep.java:49)
	at dagger.spi.internal.shaded.androidx.room.compiler.processing.XProcessingStep.process(XProcessingStep.kt:57)
	at dagger.spi.internal.shaded.androidx.room.compiler.processing.CommonProcessorDelegate.processRound(XBasicAnnotationProcessor.kt:134)
	at dagger.spi.internal.shaded.androidx.room.compiler.processing.ksp.KspBasicAnnotationProcessor.process(KspBasicAnnotationProcessor.kt:62)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$8$1.invoke(KotlinSymbolProcessingExtension.kt:320)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension$doAnalysis$8$1.invoke(KotlinSymbolProcessingExtension.kt:318)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.handleException(KotlinSymbolProcessingExtension.kt:430)
	at com.google.devtools.ksp.AbstractKotlinSymbolProcessingExtension.doAnalysis(KotlinSymbolProcessingExtension.kt:318)

[bcorso]

I looked at deploy-dagger.sh, but when I do it, for a half minute I had com.google.common.graph loaded into the -spi artifact, but not re-packaged for some reason. I tried something else that didn't work, and when I went back, it wasn't even doing that again. I'll give it another go.

You will also need to add "//third_party/java/guava" to the shaded deps here:

https://github.com/google/dagger/blob/master/dagger-spi/BUILD#L80

I think it's that the BaseGraph class is loaded by the KSP environment somewhere, in its own classloader, and the Traverser isn't, but then Traverser gets loaded in the Memory class loader by Dagger, from within the KSP environment, and it can't load from the parent because it ends up crossing java9 module boundaries.

Hmm, so is that working as intended? It kind of seems like a bug in how rules_kotlin/KSP handles its dependencies/classloading. Should the dependency within that environment be shaded instead so that at least it's only shaded once rather than requiring every processor to shade its own version of guava?

[bcorso]

Also, it definitely is running from the ksp plugin.

Hmm, perhaps the kt_ksp_plugin just gets the plugin via the META_INF info and ignores the processor_class attribute because dagger.internal.codegen.ComponentProcessor is definitely not a KSP processor class.

[bcorso]

Btw, thinking about it more I understand better now what you're trying to do but I don't think it's going to work.

First, Dagger currently only allows you to shade an entire dependency not a single package. This is because we merge entire jars, so adding //third_party/java/guava to the shaded_deps list will merge the entire guava jar. Thus, if you only shade the com.google.common.graph package we would still include the other non-shaded classes in Dagger's artifact which could cause some 1-version violation issues.

Also, IIUC, the reason you only want to shade the com.google.common.graph package is because Dagger exposes guava (e.g. ImmutableSet) in its public API (e.g. BindingGraph#bindings()) so we don't want to shade those publicly exposed classes. This also seems like another reason we shouldn't be trying to shade Guava in Dagger altogether. In particular, what if the same classloading issue starts happening in parts of the Guava API that Dagger exposes? Or what if we want to start exposing parts of the com.google.common.graph APIs via Dagger's API? Either way we'd be stuck between the classloading issues and not being able to shade the classes in our public API.

[cgruber]

Hmm, perhaps the kt_ksp_plugin just gets the plugin via the META_INF info and ignores the processor_class attribute because dagger.internal.codegen.ComponentProcessor is definitely not a KSP processor class.

I suspect it will register processor class, but if it's wrong, it'll fall back to META-INF. Either way, I changed it to point at the KSP processor and it was the same error, with the same stack-trace, so somehow it picked up the right thing.

currently only allows you to shade an entire dependency not a single package

Yeah, I worked that out last night. It's frustrating. I'm still made at Kevin B for making me abandon my "break guava into packages" plan in 2013.

I agree we can't shade all of Guava. Parts of it are in the public API as you point out.

Ultimately the shading was a holding action not a proper fix. It was intended to unblock since com.google.common.graph is a leaf-dependency, and wasn't exposed to the API (I consider the SPI distinct from the API - no runtime dependency on Guava should exist for Dagger users, unless they themselves are binding to guava types). But you're right, you might expose c.g.c.graph at some point. I was looking for a way to stop teh breakage until we could fix the classloading thing, as it seemed simpler. (at the time).

Sigh.

[cgruber]

The weird thing, is that it does work in some configurations, at some versions of these various pieces. The above mentioned PR to add java output processing used Dagger specifically as the example, and not like, an ancient version. I may have to thread the needle and find the dep versions that do comingle cleanly as my stop-gap.

[cgruber]

Hmm, so is that working as intended? It kind of seems like a bug in how rules_kotlin/KSP handles its dependencies/classloading. Should the dependency within that environment be shaded instead

Maybe, but I don't know or have connections to the KSP project proper, and so it wasnt' my first thought for a solution. But yeah, it probably should shade its usage, since I don't think IT exposes guava in any APIs.