Spring Remove call to requestCompleted in RequestMappingHandlerAdapter

RequestMappingHandlerAdapter calls webRequest.requestCompleted(); even though the async request has not completed.

Is that intentional, or is it a bug?

https://github.com/spring-projects/spring-framework/blob/dd3e3b2989bb5fc3148298c70bdb183e44597355/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerAdapter.java#L906

Comment From: rstoyanchev

This seems to have been added as part of 698f923fc3178bb9a35e1cb34c388d8703e45b99 but appears to be completely unrelated to the change, and other usages of this method are in more global places like FrameworkServlet, RequestContextFilter, and RequestContextListener. So it looks out of place in RequestMappingHandlerAdapter, and it's not clear why it was added. Removing it also does not cause any test failures.

In terms of async requests, webRequest.requestCompleted is called in FrameworkServlet as well after the initial request dispatch exits, and that is also before the overall async request has completed. In that sense the lifecycle of RequestAttributes is per request dispatch, rather than the overall request.

Do you have a specific issue that you are experiencing?

Aside from that we should probably remove this call from RequestMappingHandlerAdapter. @jhoeller what do you think?

Comment From: wplong11

@rstoyanchev I want to use RequestScope bean in a coroutine. For this, I created a SpringContext that restores the value in ThreadLocal when the Thread is changed. But ScopeNotActiveException was occured.

2022-09-19 12:08:38.976 ERROR 43080 --- [nio-8081-exec-2] o.a.c.c.C.[.[.[/].[dispatcherServlet]    : Servlet.service() for servlet [dispatcherServlet] in context with path [] threw exception [Request processing failed; nested exception is org.springframework.beans.factory.support.ScopeNotActiveException: Error creating bean with name 'scopedTarget.testService2': Scope 'request' is not active for the current thread; consider defining a scoped proxy for this bean if you intend to refer to it from a singleton; nested exception is java.lang.IllegalStateException: Cannot ask for request attribute - request is not active anymore!] with root cause

java.lang.IllegalStateException: Cannot ask for request attribute - request is not active anymore!
    at org.springframework.web.context.request.ServletRequestAttributes.getAttribute(ServletRequestAttributes.java:149) ~[spring-web-5.3.22.jar:5.3.22]
    at org.springframework.web.context.request.AbstractRequestAttributesScope.get(AbstractRequestAttributesScope.java:43) ~[spring-web-5.3.22.jar:5.3.22]
    at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:371) ~[spring-beans-5.3.22.jar:5.3.22]
    at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:208) ~[spring-beans-5.3.22.jar:5.3.22]
    at org.springframework.aop.target.SimpleBeanTargetSource.getTarget(SimpleBeanTargetSource.java:35) ~[spring-aop-5.3.22.jar:5.3.22]
    at org.springframework.aop.framework.CglibAopProxy$DynamicAdvisedInterceptor.intercept(CglibAopProxy.java:692) ~[spring-aop-5.3.22.jar:5.3.22]
    at com.example.coroutinemvctest.controller.TestService2$$EnhancerBySpringCGLIB$$b66ed748.getId(<generated>) ~[main/:na]
    at com.example.coroutinemvctest.controller.TestController2$hello$2.invokeSuspend(TestController2.kt:23) ~[main/:na]
    at kotlin.coroutines.jvm.internal.BaseContinuationImpl.resumeWith(ContinuationImpl.kt:33) ~[kotlin-stdlib-1.6.21.jar:1.6.21-release-334(1.6.21)]
    at kotlinx.coroutines.DispatchedTaskKt.resume(DispatchedTask.kt:234) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.DispatchedTaskKt.resumeUnconfined(DispatchedTask.kt:190) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.DispatchedTaskKt.dispatch(DispatchedTask.kt:161) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.CancellableContinuationImpl.dispatchResume(CancellableContinuationImpl.kt:397) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl(CancellableContinuationImpl.kt:431) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.CancellableContinuationImpl.resumeImpl$default(CancellableContinuationImpl.kt:420) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.CancellableContinuationImpl.resumeUndispatched(CancellableContinuationImpl.kt:518) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.EventLoopImplBase$DelayedResumeTask.run(EventLoop.common.kt:500) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.EventLoopImplBase.processNextEvent(EventLoop.common.kt:284) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at kotlinx.coroutines.DefaultExecutor.run(DefaultExecutor.kt:108) ~[kotlinx-coroutines-core-jvm-1.6.4.jar:na]
    at java.base/java.lang.Thread.run(Thread.java:833) ~[na:na]

package com.example.coroutinemvctest.controller

import kotlinx.coroutines.ThreadContextElement
import kotlinx.coroutines.delay
import kotlinx.coroutines.withContext
import org.springframework.stereotype.Service
import org.springframework.web.bind.annotation.GetMapping
import org.springframework.web.bind.annotation.RestController
import org.springframework.web.context.annotation.RequestScope
import org.springframework.web.context.request.AbstractRequestAttributes
import org.springframework.web.context.request.RequestAttributes
import org.springframework.web.context.request.RequestContextHolder
import kotlin.coroutines.AbstractCoroutineContextElement
import kotlin.coroutines.CoroutineContext

@RestController
class TestController(
    private val testService: TestService,
) {
    @GetMapping("/hello")
    suspend fun hello(): String = withContext(SpringContext()){
        delay(100)
        return@withContext testService.id.toString()
    }
}

private var counter = 0

@RequestScope
@Service
class TestService {
    val id: Int = ++counter
}

class SpringContext(
    private val requestAttributes: RequestAttributes? = RequestContextHolder.getRequestAttributes(),
) : ThreadContextElement<RequestAttributes?>, AbstractCoroutineContextElement(Key) {
    companion object {
        object Key : CoroutineContext.Key<SpringContext>
        private val requestActiveField = AbstractRequestAttributes::class.java.getDeclaredField("requestActive").apply {
            isAccessible = true
        }
    }

    override fun updateThreadContext(context: CoroutineContext): RequestAttributes? {
        val oldState = RequestContextHolder.getRequestAttributes()
        requestAttributes?.let {
            RequestContextHolder.setRequestAttributes(it)
            // requestActiveField.set(it, true) // if uncomment this line, then ScopeNotActiveException not occurs
        }

        return oldState
    }

    override fun restoreThreadContext(context: CoroutineContext, oldState: RequestAttributes?) {
        RequestContextHolder.setRequestAttributes(oldState)
    }
}

Comment From: rstoyanchev

I'll go ahead and remove the call to webRequest.requestCompleted() in RequestMappingHandlerAdapter that doesn't seem to have a good reason to be there.

That said, the same call will be still be made a little later in FrameworkServlet, on the way out from the initial REQUEST dispatch, and that does mean that you'll likely get the same result in your scenario.

Generally, request scoped beans rely on ThreadLocal to access the request, via RequestContextHolder, and that works only while on a Servlet container thread. For asynchronous request handling, we recommend keeping the objects you need in the controller and passing them to the methods that need them.

Comment From: firekcc

I have slove this problem，details please see Cannot ask for request attribute - request is not active anymore!