-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
CDI request scope deactivation does not work on duplicated context #37741
Comments
cc @sberyozkin related to #37472 |
Hello @cescoffier @mkouba @Ladicek @manovotn I had a look into this and I found out that CDI request scope is never activated which is for some reason expected Line 59 in 7d15333
I have updated this issue description with brand new reproducer and I propose to drop that line completely. I'll try to drop it, create test and see what Quarkus CI has to say, but I am pretty sure the PR will be rejected. Why is it implemented this way? I'd like to be able to terminate CDI request context even on duplicated context as well. |
I debugged RESTEasy Classic and I think there is another underlying issue that makes things happen only because deactivation is not actually happening (due to this bug), I'll leave experts to deal with it. Please also check how many times request context in |
Adjusted labels because this is the Arc bug - Jakarta CDI 4.0 specification in 2.5.5.2. Activating Built In Contexts subsection Activating a Request Context says that |
Keep in mind that the I did not look into the details of this issue though. Does the method return an async type, e.g. |
I appreciate you read my comment @mkouba , this is not urgent at all, but I was wondering if you know about it :-)
I used I re-tested it yesterday after #37982 got merged and I still can reproduce it.
This also happens when something is returned in a synchronous way. That said, I tried to fix it and it is really complex in scenarios like RESTEasy Classic, there can be something I'm missing. I just think deactivation should work, otherwise it's misleading operation. |
I tried looking into history to see why we chose to never remove req. context state from the vertx duplicated context and the only link I found was this comment... which sadly doesn't tell me much. Maybe
Note that |
Got it, thank you. |
Describe the bug
my use case:
Currently when gRPC extension is present, security events must be disabled or requests never reach gRPC server endpoint.
When I fire CDI event synchronously or asynchronously, I always get warning log message
Request context already active when gRPC request started
logged several times.When CDI event is fired asynchronously and the CDI request context is activated, it should not be detectable by
Arc.container.request().isActive()
, because if other code re-use the same CDI request context that is also asynchronously deactivated byio.quarkus.arc.impl.EventImpl
, than other code (like the gRPC one) will try to use deactivated context.Expected behavior
CDI request scope deactivation actually deactivates request context.
Actual behavior
Request always goes here
quarkus/extensions/grpc/runtime/src/main/java/io/quarkus/grpc/runtime/supports/context/GrpcRequestContextGrpcInterceptor.java
Line 46 in 5290dc7
How to Reproduce?
Reproducer:
Another Reproducer :
command
mvn clean verify -DskipTests -f extensions/grpc/ && mvn clean test -f extensions/grpc/deployment/ -Dtest=GrpcAuthUsingSeparatePortTest
finally
clause.quarkus/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/EventImpl.java
Line 330 in 08bff7a
io.quarkus.grpc.auth.GrpcAuthTestBase.Service#unaryCall
it is never reached (or logged):And now, you need to ask yourself how do I know it is also related to synchronous events here - you can just comment out this line,
quarkus/extensions/security/runtime-spi/src/main/java/io/quarkus/security/spi/runtime/SecurityEventHelper.java
Line 72 in 08bff7a
build Security extension and re-run test with a same result.
Output of
uname -a
orver
Fedora 38
Output of
java -version
OpenJDK Runtime Environment Temurin-17.0.7+7
Quarkus version or git rev
999-SNAPSHOT
Build tool (ie. output of
mvnw --version
orgradlew --version
)Apache Maven 3.9.3
Additional information
No response
The text was updated successfully, but these errors were encountered: