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

Throw an exception for suspending factory methods #32719

Closed
Hansanto opened this issue Apr 27, 2024 · 4 comments
Closed

Throw an exception for suspending factory methods #32719

Hansanto opened this issue Apr 27, 2024 · 4 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement
Milestone

Comments

@Hansanto
Copy link

Hello

The @Bean seems to be called two times according to logs (below) in a suspend kotlin function.

This is my app code:

@SpringBootApplication
class SpringKaultApplication {

    @Bean
    suspend fun greeting(): String {
        println("Hello, World!")
        return "My test"
    }
}

fun main(args: Array<String>) {
    runApplication<SpringKaultApplication>(*args)
}

And the logs when I started the application:

2024-04-27T18:14:29.228+00:00  INFO 27824 --- [           main] o.e.s.SpringApplicationKt           : Starting SpringApplicationKt using Java 19.0.2
2024-04-27T18:14:29.230+00:00  INFO 27824 --- [           main] o.e.s.SpringApplicationKt           : No active profile set, falling back to 1 default profile: "default"
Hello, World!
Hello, World!
2024-04-27T18:14:30.566+00:00  INFO 27824 --- [           main] o.s.b.web.embedded.netty.NettyWebServer  : Netty started on port 8080
2024-04-27T18:14:30.572+00:00  INFO 27824 --- [           main] o.e.s.SpringApplicationKt           : Started SpringApplicationKt in 1.663 seconds (process running for 2.204)

Now, if I try without the suspend keyword like:

@SpringBootApplication
class SpringApplication {

    @Bean
    fun greeting(): String {
        println("Hello, World!")
        return "My test"
    }
}

fun main(args: Array<String>) {
    runApplication<SpringApplication>(*args)
}

And the logs:

2024-04-27T18:21:41.495+00:00  INFO 27660 --- [           main] o.e.s.SpringApplicationKt        : Starting SpringApplicationKt using Java 19.0.2
2024-04-27T18:21:41.497+00:00  INFO 27660 --- [           main] o.e.s.SpringApplicationKt        : No active profile set, falling back to 1 default profile: "default"
Hello, World!
2024-04-27T18:21:42.936+00:00  INFO 27660 --- [           main] o.s.b.web.embedded.netty.NettyWebServer  : Netty started on port 8080
2024-04-27T18:21:42.942+00:00  INFO 27660 --- [           main] o.e.s.SpringApplicationKt        : Started SpringApplicationKt in 1.755 seconds (process running for 2.226)

There are the other file to reproduces:

build.gradle.kts

import org.jetbrains.kotlin.gradle.tasks.KotlinCompile

plugins {
    id("org.springframework.boot") version "3.2.5"
    id("io.spring.dependency-management") version "1.1.4"
    kotlin("jvm") version "1.9.23"
    kotlin("plugin.spring") version "1.9.23"
}

group = "org.example"
version = "0.0.1-SNAPSHOT"

java {
    sourceCompatibility = JavaVersion.VERSION_17
}

repositories {
    mavenCentral()
}

dependencies {
    implementation("org.springframework.boot:spring-boot-starter-webflux")
    implementation("io.projectreactor.kotlin:reactor-kotlin-extensions")
    implementation("org.jetbrains.kotlin:kotlin-reflect")
    implementation("org.jetbrains.kotlinx:kotlinx-coroutines-reactor")
    testImplementation("org.springframework.boot:spring-boot-starter-test")
    testImplementation("io.projectreactor:reactor-test")
}

tasks.withType<KotlinCompile> {
    kotlinOptions {
        freeCompilerArgs += "-Xjsr305=strict"
        jvmTarget = "17"
    }
}

tasks.withType<Test> {
    useJUnitPlatform()
}

Version:

  • Java: 19.0.2
  • Kotlin: 1.9.23
  • Spring boot: 3.2.5
  • Gradle: 8.7
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Apr 27, 2024
@snicoll snicoll changed the title [BUG]: Bean called two times in kotlin suspend context Bean called two times in kotlin suspend context Apr 27, 2024
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support labels Apr 29, 2024
@sdeleuze sdeleuze self-assigned this Apr 29, 2024
@sdeleuze sdeleuze changed the title Bean called two times in kotlin suspend context Throw an exception for suspending factory methods Apr 29, 2024
@sdeleuze sdeleuze added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Apr 29, 2024
@sdeleuze sdeleuze added this to the 6.2.0-M2 milestone Apr 29, 2024
@sdeleuze
Copy link
Contributor

sdeleuze commented Apr 29, 2024

Suspending functions should not be used with @Bean and for factory methods, Spring Framework 6.2+ is going to throw an exception for such use case (including when AOT is used).

FYI, in your repro, the message is printed 2 times because the decompiled generated bytecode is:

public class DemoSbWithValueClassApplication {
   @Bean
   @Nullable
   public Object greeting(@NotNull Continuation $completion) {
      return greeting$suspendImpl(this, $completion);
   }

   // $FF: synthetic method
   @Bean
   static Object greeting$suspendImpl(DemoSbWithValueClassApplication $this, Continuation $completion) {
      String var2 = "Hello, World!";
      System.out.println(var2);
      return "My test";
   }
}

@Hansanto
Copy link
Author

I don't understand why it's a problem to instantiate a bean in a suspend method.
Instead of throwing an exception, why not fix it to allow this feature?

In Kotlin, some libraries create a builder using a suspend method.
Consequently, if an exception is thrown, you will block the possibility of using this type of builder.

I suggest creating the Bean using a coroutine attached to the main thread (or the dedicated thread where the Bean are instantiated).
Of course, that implies checking if the function is a kotlin function, duplicated (one with the basic args and another with basic args & Continuation object (from coroutine)) and consequently load only matching Bean.

For example (that's not the best example, but maybe it will allow you to understand the need and how to do that in Kotlin):

import kotlin.properties.Delegates
import kotlinx.coroutines.delay

public class MyClient(
    public val arg1: Int,
    public val arg2: String,
) {

    public companion object {

        /**
         * ```kotlin
         * val client = MyClient {
         *   arg1 = 1
         *   // ...
         * }
         * ```
         */
        public suspend inline operator fun invoke(builder: Builder.() -> Unit): MyClient =
            Builder().apply(builder).build()
    }

    public class Builder {

        public var arg1: Int by Delegates.notNull()

        public lateinit var arg2: String

        public var shouldCallMyServer: Boolean = false

        public suspend fun build(): MyClient {
            return MyClient(
                arg1 = arg1,
                arg2 = arg2
            ).apply {
                // Init the client with suspending calls according to some builder properties
                if (shouldCallMyServer) {
                    // Simulate a suspending call
                    delay(1000)
                }
            }
        }
    }
}

With that, if I need to do something that needs I/O operation, according to the builder properties and to simplify the management of my object, I can do this in the builder.

@Hansanto
Copy link
Author

After of course, you can wrap your Builder in a run blocking method.
So we already have an option to do that without putting the suspend keyword on the function signature.

Like:

@SpringBootApplication
class SpringApplication {

    @Bean
    fun greeting(): MyClient {
        return runBlocking {
           MyClient { ... }
        }
    }
}

sdeleuze added a commit to sdeleuze/spring-framework that referenced this issue May 10, 2024
Suspending factory methods are not supported, and can
have side effects, so it is better to fail explicitly
for such use case.

Closes spring-projectsgh-32719
@sdeleuze
Copy link
Contributor

I don't think suspending factory method conceptually make sense, they have side effects, and you indeed have the possibility to use runBlocking { } if you really need this, so Spring Framework 6.2+ will explicitly fail for such use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants