Since 3.10.1 RequestContext handling is broken on request cancle #40810
Comments
gsmet
added
area/arc
|
@gsmet Yes, sorry I've mixed it up, that is the one I was talking about. |
|
@HerrDerb Maybe I don't fully understand it but the use case seems a bit weird. My thoughts on it are below; feel free to express disagreement and/or clarify the use case.
On a req. timeout, it seems sensible to destroy req. context as past that point we don't know when or if at all it will get destroyed.
And more importantly this part you wrote - even if the req. context was re-created, it was a fresh one and therefore any such beans lost their state in the meantime. That is IMO an indicator that your bean shouldn't be req. scoped to begin with. Maybe app scoped or dependent? That way you aren't linking it to lifecycle of a req. that you suspect might end prematurely. |
|
@gsmet When we talked about this issue I missed the "request timeout" part. I think that I agree with Matej that the request context should be destroyed after the timeout and that the |
|
Man, that is hard to describe 🙂 I understand that right now, when Vert.x receives a request cancel or timeout, it destroys its Before 3.10.1, the dispatched process had its own So what I am suggesting is: While reusing the With the change of 3.10.1 this leads to unexected behaviour. Does this seem understandable? |
|
For me the interesting part is, that adding a simple In my opinion, the behavior should be the same with and without an empty vertx filter and I tend to the plain behavior currently happening without the filter. |
I'm not an expert in this area but AFAIK you cannot easily interrupt a running thread unless the code checks
Yes, and we identified this as an incorrect behavior 🤷.
I tend to disagree but we should probably discuss this more broadly. CC @geoand
I do understand your expectations but I think that your use case worked just by accident.
It's interesting but not surprising. Because when a |
I agree with this |
|
With this reasoning, following thoughts occur to me:
|
|
I tried debugging it some more to better see what's going on.
You are expecting a
I agree this does seem weird but I am not sure what we can do. |
Yes, and that the vert.x owned request context does ignore depending processes which are still running. (by ignoring I mean it does not wait nor cancel any depending running processes) Well I don't really expect a timeout. The reproducer is just build like this to provoke the issue. A http request can be cancelled out of different reasons: timeout by the client, closing web site, cancle request with angular To clarify our productive use case: |
|
I am not sure if it is the same problem but we also get a Since Quarkus 3.10 we get the following exception (Note: The second error is a follow-up error of the We do not explicity use I am pretty sure that there is a bug in Quarkus because the endpoint is quite simple. The endpoint only generates vector tiles for display on a map, it does not access a database or an external service, but there are a lot of requests. I have not yet managed to create a reproducer. |
|
@mschorsch can you specify the exact version you are using? |
Well, it's not that surprising since you mix multiple technologies here. The Now if you really need a "plain" filter that controls the CDI request context activation, you can use the public void init(@Observes Filters filters, RequestScopedBean requestScopedBean, RequestContextController controller) {
filters.register(rc -> {
try {
controller.activate();
requestScopedBean.doSomething();
} finally {
controller.deactivate();
}
rc.next();
}, 10); }
}But obviously you'll have two separate request contexts activated during a specific HTTP request. Which is IMO not a good idea.
|
|
As I understand quarkus-rest is built on quarkus-vertx-http, so I would have expected they work together. A odd discovery we just made with your input by not mixing Here are the logs of the different scenarios generated with my reproducer, feel free to try yourself. The uuid of the RequestScopedBean is per instance, hinting thecurrent managed context. As it is, with VertX RouteFilter and ContainerRequestFilter(witht @PreMatching): Context gets reused -> Test failes With VertX RouteFilter and ContainerRequestFilter(without @PreMatching): Context is not reused -> Test succeeds Only With VertX RouteFilter: Context is not reused -> Test succeeds |
Keep in mind that |
Quarkus 3.11 but after reading the new comments i am pretty sure that my issue is different from this one. I will open a new Issue when i have a reproducer. |
|
@mkouba I wasn't aware that combining those two is an issue, thanks. With a plain VertX filter it works as expected and it is also one dependency less. Final thoughts: |
Describe the bug
Issue was introduced with Quarkus REST - reuse CDI request context if it exists
#40307 (comment)
Well I think we found a constellation:
There is a Vertx RouteFilter and a ContainerRequestFilter which both access a
@RequestScopedbean.The logic of the reproducer application executes an uninterruptable process which takes a while (in this case a db operation).
The test provokes a request timeout hence a request cancle.
On a request cancle, the verx request context gets destroyed immediately but the uninterruptable code keeps executing and finally triggers a
ContextNotActiveException.Before 3.10.1 this wasn't an issue as the
ContainerRequestFiltercreated a new request context, and therefor was not affected by a request cancle.Expected behavior
On a request cancle the ActiveRequestContext gets destroyed while the request code is still running
Actual behavior
Wait for code completion before destroying the context
How to Reproduce?
Reproducer: https://github.com/HerrDerb/quarkus-issue/tree/context-issue
Steps:
Output of
uname -aorverNo response
Output of
java -versionNo response
Quarkus version or git rev
No response
Build tool (ie. output of
mvnw --versionorgradlew --version)No response
Additional information
No response
The text was updated successfully, but these errors were encountered: