Using recursive annotations in Kotlin causes stack overflow #28012
Comments
spring-projects-issues
added
the
status: waiting-for-triage
|
Your description indicates Stack overflow but you haven't shared the logs. Can you do that so that we can check if that is related to Spring Boot? |
snicoll
added
the
status: waiting-for-feedback
|
Main class: @SpringBootApplication
class AnnotationDemo2Application
fun main(args: Array<String>) {
runApplication<AnnotationDemo2Application>(*args)
}
@JvmRepeatable(Bar::class)
annotation class Foo(val value: Bar = Bar())
annotation class Bar(vararg val value: Foo)
@RestController
class HelloController {
@Foo
@Foo
@GetMapping("/a")
fun hi() = "hello"
}↓ This is the entire log generated by the relevant code from the start: full debug log. ____ _ __ _ _ /\\ / ___'_ __ _ _(_)_ __ __ _ \ \ \ \ ( ( )\___ | '_ | '_| | '_ \/ _` | \ \ \ \ \\/ ___)| |_)| | | | | || (_| | ) ) ) ) ' |____| .__|_| |_|_| |_\__, | / / / / =========|_|==============|___/=/_/_/_/ :: Spring Boot :: (v2.6.3) |
spring-projects-issues
added
status: feedback-provided
|
The exception is coming from Spring Framework's meta-annotation processing code. It's interesting that Kotlin allows this: @JvmRepeatable(Bar::class)
annotation class Foo(val value: Bar = Bar())
annotation class Bar(vararg val value: Foo)With the following java code: @interface Foo {
Bar value();
}
@interface Bar {
Foo value();
}The compiler fails with "Cycle detected: a cycle exists between annotation attributes of Bar and Foo" I guess Framework will need an additional guard. |
sbrannen
added
in: core
sbrannen
changed the title
sbrannen
changed the title
Yes, if Kotlin allows that to be compiled (which I also find a bit strange), I suppose we should at least guard against infinite recursion in our annotation processing. @ForteScarlet, what is your concrete use case for declaring recursive annotations? |
|
@sbrannen @Filter("hello")
@Filter("hi")
suspend fun Event.onEvent() {
// ...
}
Also, I would like to nest additional matching logic in the @Filter("hello", or = Filters(Filter(value = "hi", user = ["123", "456"])))
suspend fun Event.onEvent() {
// ...
}Their source codes are here: https://github.com/ForteScarlet/simpler-robot/blob/3cf422e4161cd1128e60599ad2cc176ec989c47d/boots/simboot-core-annotation/src/main/kotlin/love/forte/simboot/annotation/Filter.kt |
|
@ForteScarlet, thanks for providing the concrete use case and the link to the source code. I see in the Javadoc that you have examples of using The reason I ask is that I know recursive annotations cannot be defined/compiled in Java (although it works with the Kotlin compiler), and I'm wondering if those Kotlin-compiled recursive annotations are actually usable in Java applications. In any case, we'll investigate what we can do to avoid throwing exceptions (or resulting in a stack overflow) if recursive annotations are encountered while processing annotations in Spring Framework. |
|
@sbrannen But I just went and tried again, and the code is roughly as follows: @Filter(value = "Hello.+", and = @Filters(@Filter("Hello\\d+")))
@Filter("Hi")
@Listener
public void onEvent(FriendMessageEvent event) {
System.out.println(event);
System.out.println(event.getMessageContent().getPlainText());
}In the IDE, it functions as expected and works fine. I also packaged it with the application plugin and tested it locally from my computer and it works fine. In the IDE, I simply tried to compile using the following JDKs:
🤔From the results, it appears that the recursive annotations compile and work. |
sbrannen
removed
the
status: waiting-for-triage
sbrannen
added
type: bug
Although Java does not allow the definition of recursive annotations, Kotlin does, and prior to this commit an attempt to synthesize a merged annotation using the MergedAnnotation API resulted in a StackOverflowError if there was recursive cycle in the annotation definitions. This commit addresses this issue by tracking which annotations have already been visited and short circuits the recursive algorithm if a cycle is detected. closes spring-projectsgh-28012
|
@ForteScarlet, thanks for all of your feedback! I was able to reproduce the problem and have pushed a draft implementation for a fix in sbrannen@6cd1540. |
Although Java does not allow the definition of recursive annotations, Kotlin does, and prior to this commit an attempt to synthesize a merged annotation using the MergedAnnotation API resulted in a StackOverflowError if there was a recursive cycle in the annotation definitions. This commit addresses this issue by tracking which annotations have already been visited and short circuits the recursive algorithm if a cycle is detected. Closes spring-projectsgh-28012
|
This has been fixed in @ForteScarlet, feel free to try it out in an upcoming snapshot build for |
|
@sbrannen I just tried version |
In 3ec612a, I accidentally removed tests that verified support for non-synthesizable merged annotations for recursive annotations in Kotlin. This commit reinstates those non-synthesizable tests while retaining the synthesizable tests.
|
@ForteScarlet, very glad to hear that it works for you! Thanks for trying out the snapshot and letting us know. |
|
Alas, this was not meant to be. Kotlin 1.7 gives a compiler warning for cyclic annotation parameter types, and Kotlin 1.9 will turn the warning into an error, following Java's behavior. See KT-47932. |
|
Very interesting (and dare I say good)! Thanks for sharing, @poutsma. We'll eventually have to remove those dedicated Kotlin tests from our test suite, but we can leave the "fix" in place for code that was compiled with Kotlin pre-1.9. |
spring boot version:
2.6.3I have defined several annotations in kotlin similar to the following:
This does not seem to be possible in Java, but kotlin allows it. Then I use it in springboot:
I get exceptions at startup:
The text was updated successfully, but these errors were encountered: