Only deactivate request context if it was inactive before migrating it (master)

microprofile/fault-tolerance/src/main/java/io/helidon/microprofile/faulttolerance/RequestScopeHelper.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2021, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -113,29 +113,29 @@ FtSupplier<Object> wrapInScope(FtSupplier<Object> supplier) {
             return () -> requestScope.runInScope(requestContext,
                     (Callable<?>) (() -> {
                         InjectionManager old = WeldRequestScope.actualInjectorManager.get();
-                        BoundRequestContext boundRequestContext = null;
+                        Runnable migrationCleaner = null;
                         try {
-                            boundRequestContext = migrateRequestContext();
+                            migrationCleaner = migrateRequestContext();
                             WeldRequestScope.actualInjectorManager.set(injectionManager);
                             return supplier.get();
                         } catch (Throwable t) {
                             throw t instanceof Exception ? ((Exception) t) : new RuntimeException(t);
                         } finally {
-                            if (boundRequestContext != null) {
-                                boundRequestContext.deactivate();
+                            if (migrationCleaner != null) {
+                                migrationCleaner.run();
                             }
                             WeldRequestScope.actualInjectorManager.set(old);
                         }
                     }));
         } else if (weldManager != null) {         // CDI only
             return () -> {
-                BoundRequestContext boundRequestContext = null;
+                Runnable migrationCleaner = null;
                 try {
-                    boundRequestContext = migrateRequestContext();
+                    migrationCleaner = migrateRequestContext();
                     return supplier.get();
                 } finally {
-                    if (boundRequestContext != null) {
-                        boundRequestContext.deactivate();
+                    if (migrationCleaner != null) {
+                        migrationCleaner.run();
                     }
                 }
             };
@@ -151,17 +151,32 @@ FtSupplier<Object> wrapInScope(FtSupplier<Object> supplier) {
      * if a request scope bean in the original context was not accessed/proxied, it
      * will not be carried over.
      *
-     * @return the request context
+     * @return runnable that cleans up after migration or {@code null}
      */
-    private BoundRequestContext migrateRequestContext() {
+    private Runnable migrateRequestContext() {
         if (requestScopeInstances != null) {
+            // Access CDI context instance
             BoundRequestContext requestContext = weldManager.instance()
                     .select(BoundRequestContext.class, BoundLiteral.INSTANCE).get();
+
+            // Ensure a storage and activate if necessary
             Map<String, Object> requestMap = new HashMap<>();
-            requestContext.associate(requestMap);
-            requestContext.activate();
+            boolean wasAssociated = requestContext.associate(requestMap);
             requestContext.clearAndSet(requestScopeInstances);
-            return requestContext;
+            boolean wasActive = requestContext.isActive();
+            if (!wasActive) {
+                requestContext.activate();
+            }
+
+            // Return runnable that properly cleans up after context migration
+            return () -> {
+                if (!wasActive) {
+                    requestContext.deactivate();
+                }
+                if (wasAssociated) {
+                    requestContext.dissociate(requestMap);
+                }
+            };
         }
         return null;
     }

tests/functional/context-propagation/src/main/java/io/helidon/tests/functional/context/hello/RequestReaderInterceptor.java

@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2022 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.tests.functional.context.hello;
+
+import jakarta.enterprise.context.RequestScoped;
+import jakarta.ws.rs.WebApplicationException;
+import jakarta.ws.rs.ext.Provider;
+import jakarta.ws.rs.ext.ReaderInterceptor;
+import jakarta.ws.rs.ext.ReaderInterceptorContext;
+import java.io.IOException;
+
+/**
+ * A reader interceptor in request scope.
+ *
+ * @see HelloResource#getHello
+ */
+@Provider
+@RequestScoped
+public class RequestReaderInterceptor implements ReaderInterceptor {
+
+    @Override
+    public Object aroundReadFrom(ReaderInterceptorContext context) throws IOException, WebApplicationException {
+        return context.proceed();
+    }
+}

tests/functional/context-propagation/src/main/java/io/helidon/tests/functional/context/hello/ResponseWriterInterceptor.java

@@ -0,0 +1,41 @@
+/*
+ * Copyright (c) 2022 Oracle and/or its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.helidon.tests.functional.context.hello;
+
+import jakarta.enterprise.context.RequestScoped;
+import jakarta.ws.rs.ext.Provider;
+import jakarta.ws.rs.ext.WriterInterceptor;
+import jakarta.ws.rs.ext.WriterInterceptorContext;
+import java.io.IOException;
+
+/**
+ * Presence of this interceptor in request scope tests that a request scope is
+ * active after returning from the resource method call while running in a
+ * different thread -- such as a fault tolerance thread.
+ *
+ * @see "https://github.com/oracle/helidon/issues/3804"
+ * @see HelloResource#getHello
+ */
+@Provider
+@RequestScoped
+public class ResponseWriterInterceptor implements WriterInterceptor {
+
+    @Override
+    public void aroundWriteTo(WriterInterceptorContext context) throws IOException {
+        context.proceed();
+    }
+}