Save setting the same state in the Vertx local data duplicated context

quarkusio · Jun 27, 2023 · 61bd514 · 61bd514
1 parent 01ccac6
commit 61bd514
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 23 deletions.
diff --git a/...ions/vertx/runtime/src/main/java/io/quarkus/vertx/runtime/VertxCurrentContextFactory.java b/...ions/vertx/runtime/src/main/java/io/quarkus/vertx/runtime/VertxCurrentContextFactory.java
@@ -39,7 +39,13 @@ public void set(T state) {
             Context context = Vertx.currentContext();
             if (context != null && VertxContext.isDuplicatedContext(context)) {
                 VertxContextSafetyToggle.setContextSafe(context, true);
-                context.putLocal(LOCAL_KEY, state);
+                // this is racy but should be fine, because DC should not be shared
+                // and never remove the existing mapping
+                var oldState = context.getLocal(LOCAL_KEY);
+                if (oldState != state) {
+                    context.putLocal(LOCAL_KEY, state);
+                }
+
             } else {
                 fallback.set(state);
             }

diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java
@@ -124,13 +124,7 @@ public void destroy(Contextual<?> contextual) {
     @Override
     public void activate(ContextState initialState) {
         if (LOG.isTraceEnabled()) {
-            String stack = Arrays.stream(Thread.currentThread().getStackTrace())
-                    .skip(2)
-                    .limit(7)
-                    .map(se -> "\n\t" + se.toString())
-                    .collect(Collectors.joining());
-            LOG.tracef("Activate %s %s\n\t...",
-                    initialState != null ? Integer.toHexString(initialState.hashCode()) : "new", stack);
+            traceActivate(initialState);
         }
         if (initialState == null) {
             currentContext.set(new RequestContextState(new ConcurrentHashMap<>()));
@@ -145,6 +139,16 @@ public void activate(ContextState initialState) {
         }
     }
 
+    private void traceActivate(ContextState initialState) {
+        String stack = Arrays.stream(Thread.currentThread().getStackTrace())
+                .skip(2)
+                .limit(7)
+                .map(se -> "\n\t" + se.toString())
+                .collect(Collectors.joining());
+        LOG.tracef("Activate %s %s\n\t...",
+                initialState != null ? Integer.toHexString(initialState.hashCode()) : "new", stack);
+    }
+
     @Override
     public ContextState getState() {
         RequestContextState state = currentContext.get();
@@ -162,16 +166,20 @@ public ContextState getStateIfActive() {
     @Override
     public void deactivate() {
         if (LOG.isTraceEnabled()) {
-            String stack = Arrays.stream(Thread.currentThread().getStackTrace())
-                    .skip(2)
-                    .limit(7)
-                    .map(se -> "\n\t" + se.toString())
-                    .collect(Collectors.joining());
-            LOG.tracef("Deactivate%s\n\t...", stack);
+            traceDeactivate();
         }
         currentContext.remove();
     }
 
+    private static void traceDeactivate() {
+        String stack = Arrays.stream(Thread.currentThread().getStackTrace())
+                .skip(2)
+                .limit(7)
+                .map(se -> "\n\t" + se.toString())
+                .collect(Collectors.joining());
+        LOG.tracef("Deactivate%s\n\t...", stack);
+    }
+
     @Override
     public void destroy() {
         destroy(currentContext.get());
@@ -180,12 +188,7 @@ public void destroy() {
     @Override
     public void destroy(ContextState state) {
         if (LOG.isTraceEnabled()) {
-            String stack = Arrays.stream(Thread.currentThread().getStackTrace())
-                    .skip(2)
-                    .limit(7)
-                    .map(se -> "\n\t" + se.toString())
-                    .collect(Collectors.joining());
-            LOG.tracef("Destroy %s%s\n\t...", state != null ? Integer.toHexString(state.hashCode()) : "", stack);
+            traceDestroy(state);
         }
         if (state == null) {
             // nothing to destroy
@@ -210,6 +213,15 @@ public void destroy(ContextState state) {
         }
     }
 
+    private static void traceDestroy(ContextState state) {
+        String stack = Arrays.stream(Thread.currentThread().getStackTrace())
+                .skip(2)
+                .limit(7)
+                .map(se -> "\n\t" + se.toString())
+                .collect(Collectors.joining());
+        LOG.tracef("Destroy %s%s\n\t...", state != null ? Integer.toHexString(state.hashCode()) : "", stack);
+    }
+
     private void destroyContextElement(Contextual<?> contextual, ContextInstanceHandle<?> contextInstanceHandle) {
         try {
             contextInstanceHandle.destroy();
@@ -236,6 +248,14 @@ private ContextNotActiveException notActive() {
 
     static class RequestContextState implements ContextState {
 
+        // Using 0 as default value enable removing an initialization
+        // in the constructor, piggybacking on the default value.
+        // As per https://docs.oracle.com/javase/specs/jls/se8/html/jls-12.html#jls-12.5
+        // the default field values are set before 'this' is accessible, hence
+        // they should be the very first value observable even in presence of
+        // unsafe publication of this object.
+        private static final int VALID = 0;
+        private static final int INVALID = 1;
         private static final VarHandle IS_VALID;
 
         static {
@@ -251,7 +271,6 @@ static class RequestContextState implements ContextState {
 
         RequestContextState(ConcurrentMap<Contextual<?>, ContextInstanceHandle<?>> value) {
             this.map = Objects.requireNonNull(value);
-            this.isValid = 1;
         }
 
         @Override
@@ -265,12 +284,12 @@ public Map<InjectableBean<?>, Object> getContextualInstances() {
          */
         boolean invalidate() {
             // Atomically sets the value just like AtomicBoolean.compareAndSet(boolean, boolean)
-            return IS_VALID.compareAndSet(this, 1, 0);
+            return IS_VALID.compareAndSet(this, VALID, INVALID);
         }
 
         @Override
         public boolean isValid() {
-            return isValid == 1;
+            return isValid == VALID;
         }
 
     }