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

Support invoking bridged suspending functions in AopUtils #33045

Closed
Vano2776 opened this issue Jun 17, 2024 · 6 comments
Closed

Support invoking bridged suspending functions in AopUtils #33045

Vano2776 opened this issue Jun 17, 2024 · 6 comments
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support type: bug A general bug
Milestone

Comments

@Vano2776
Copy link

Vano2776 commented Jun 17, 2024

Thrown java.lang.NullPointerException when call method with generic type parameters from proxied class.

Case:

interface A <T> {
    suspend fun f1(a: T)
    suspend fun f2(a: T)
}

class SomeClass

@Component
class B : A<SomeClass> {

    override suspend fun f1(a: SomeClass) {}

    @Transactional
    override suspend fun f2(a: SomeClass) {}
}

@Service
class Service(
    private val component: A<SomeClass>
) {
    suspend fun test() {
        component.f2(SomeClass()) // call success
        component.f1(SomeClass()) // throw java.lang.NullPointerException
    }
}

Stack trace:

java.lang.NullPointerException: null at java.base/java.util.Objects.requireNonNull(Objects.java:233) at
org.springframework.core.CoroutinesUtils.invokeSuspendingFunction(CoroutinesUtils.java:111) at
org.springframework.aop.support.AopUtils$KotlinDelegate.invokeSuspendingFunction(AopUtils.java:376) at
org.springframework.aop.support.AopUtils.invokeJoinpointUsingReflection(AopUtils.java:351) at
org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:713)

Example solve:
add bridge method resolve in

retVal = AopUtils.invokeJoinpointUsingReflection(target, method, argsToUse);

...
if (chain.isEmpty()) {
    // We can skip creating a MethodInvocation: just invoke the target directly.
    // Note that the final invoker must be an InvokerInterceptor, so we know
    // it does nothing but a reflective operation on the target, and no hot
    // swapping or fancy proxying.
    Object[] argsToUse = AopProxyUtils.adaptArgumentsIfNecessary(method, args);
    retVal = AopUtils.invokeJoinpointUsingReflection(target, BridgeMethodResolver.findBridgedMethod(method), argsToUse);
}
else {
    // We need to create a method invocation...
    retVal = new CglibMethodInvocation(proxy, target, method, args, targetClass, chain, methodProxy).proceed();
}
...

Affects: <Spring Framework 6.1, Spring Boot 3.2.0>

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Jun 17, 2024
@jhoeller jhoeller added in: core Issues in core modules (aop, beans, core, context, expression) theme: kotlin An issue related to Kotlin support labels Jun 17, 2024
@sdeleuze sdeleuze self-assigned this Jun 17, 2024
@sdeleuze
Copy link
Contributor

Could you please check with the latest Boot 3.2 patch release available and provide a self contained reproducer (link to a repository or an attached archive)?

@sdeleuze sdeleuze added the status: waiting-for-feedback We need additional information before we can continue label Jun 18, 2024
@cloudchamb3r
Copy link

Same issue on Spring Boot Version 3.3

@SpringBootApplication
open class BuggyKotlinApplication

open class SomeClass()

open interface A<T> {
    suspend fun nonTransactional(a: T)
    suspend fun transactional(a: T)
}


@Component
open class B : A<SomeClass> {
    val logger = org.slf4j.LoggerFactory.getLogger(this::class.java)

    override suspend fun nonTransactional(a: SomeClass) {
        logger.info("f1 called")
    }

    @Transactional
    override suspend fun transactional(a: SomeClass) {
        logger.info("f2 called")
    }
}

@Service
class SomeService(
    private val component : A<SomeClass>
) {
    suspend fun test() {
        component.nonTransactional(SomeClass())
    }

    @Transactional
    suspend fun test2() {
        component.transactional(SomeClass())
    }
}

@RestController
class SomeController(
    private val service: SomeService
) {
    @GetMapping("/")
    suspend fun test(): String {
        service.test()
        return "test"
    }
    @GetMapping("/tx")
    suspend fun tx(): String {
        service.test2()
        return "tx"
    }

    @GetMapping("/both")
    suspend fun both(): String {
        service.test()
        service.test2()
        return "both"
    }

    @GetMapping("/both-rev")
    suspend fun bothRev(): String {
        service.test2()
        service.test()
        return "both-rev"
    }
}

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

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Jun 18, 2024
@gitdude1458
Copy link

gitdude1458 commented Jul 9, 2024

The issue seems to exists in all 3.2.x and 3.3.x branches. The latest version I could not reproduce this was 3.1.12.
Notable change of Spring Boot 3.2.x is Kotlin 1.9.x (previously 1.8.x), however downgrading Kotlin back to 1.8.x does not seem to affect the issue in any way.

Notable change from 3.1.x to 3.2.0 was introduction of AOP support for Kotlin coroutines: #22462

My guess is that the previous support was somewhat limited / broken (it worked properly until the coroutine suspended first - which would break @Transactional for sure, but has been enough for aspects like @PreAuthorize that are fully executed before first suspend).

I understand the AOP support is somewhat limited as indicated by the issue, however for aspects like @PreAuthorize this is unfortunately regression.

@sdeleuze
Copy link
Contributor

Please provide a self-contained reproducer (link to a repository or an attached archive) as asked above.

@sdeleuze sdeleuze added status: waiting-for-feedback We need additional information before we can continue and removed status: feedback-provided Feedback has been provided labels Jul 15, 2024
@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Jul 22, 2024
@Vano2776
Copy link
Author

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue status: feedback-reminder We've sent a reminder that we need additional information before we can continue labels Jul 28, 2024
@sdeleuze sdeleuze added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on status: feedback-provided Feedback has been provided labels Aug 12, 2024
@sdeleuze sdeleuze added this to the 6.1.12 milestone Aug 12, 2024
@sdeleuze sdeleuze changed the title Bug in spring-aop 6.1 with kotlin and generic types. Support invoking bridged suspending functions in AopUtils Aug 12, 2024
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: bug A general bug
Projects
None yet
Development

No branches or pull requests

6 participants