Correctly ensure ordering via VertxServiceProvider spi + optimize int…

…erceptors
quarkusio · May 24, 2024 · cc907b4 · cc907b4
1 parent 48f27cd
commit cc907b4
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 15 deletions.
diff --git a/extensions/vertx/runtime/src/main/java/io/quarkus/vertx/core/runtime/QuarkusAccessModes.java b/extensions/vertx/runtime/src/main/java/io/quarkus/vertx/core/runtime/QuarkusAccessModes.java
@@ -3,27 +3,37 @@
 import java.util.concurrent.atomic.AtomicReferenceArray;
 import java.util.function.Supplier;
 
+import io.vertx.core.Context;
 import io.vertx.core.spi.context.storage.AccessMode;
 
 final class QuarkusAccessModes {
 
+    /**
+     * Beware this {@link AccessMode#getOrCreate(AtomicReferenceArray, int, Supplier)} because, differently from
+     * {@link io.vertx.core.spi.context.storage.ContextLocal#get(Context, Supplier)},
+     * is not suitable to be used with {@link io.vertx.core.spi.context.storage.ContextLocal#get(Context, AccessMode, Supplier)}
+     * with the same guarantees of atomicity i.e. the supplier can get called more than once by different racing threads!
+     */
     public static final AccessMode ACQUIRE_RELEASE = new AccessMode() {
         @Override
         public Object get(AtomicReferenceArray<Object> locals, int idx) {
-            return locals.get(idx);
+            return locals.getAcquire(idx);
         }
 
         @Override
         public void put(AtomicReferenceArray<Object> locals, int idx, Object value) {
-            locals.lazySet(idx, value);
+            // This is still ensuring visibility across threads and happens-before,
+            // but won't impose setVolatile total ordering i.e. StoreLoad barriers after write
+            // to make it faster
+            locals.setRelease(idx, value);
         }
 
         @Override
         public Object getOrCreate(AtomicReferenceArray<Object> locals, int idx, Supplier<Object> initialValueSupplier) {
-            Object value = locals.get(idx);
+            Object value = locals.getAcquire(idx);
             if (value == null) {
                 value = initialValueSupplier.get();
-                locals.lazySet(idx, value);
+                locals.setRelease(idx, value);
             }
             return value;
         }

diff --git a/extensions/vertx/runtime/src/main/java/io/quarkus/vertx/core/runtime/VertxLocalsHelper.java b/extensions/vertx/runtime/src/main/java/io/quarkus/vertx/core/runtime/VertxLocalsHelper.java
@@ -24,10 +24,11 @@ public static void throwOnRootContextAccess() {
     public static <T> T getLocal(ContextInternal context, Object key) {
         if (VertxContext.isDuplicatedContext(context)) {
             // We are on a duplicated context, allow accessing the locals
-            if (key instanceof ContextLocalImpl<?> || key instanceof ContextLocal<?>) {
+            if (key instanceof ContextLocalImpl<?>) {
                 var localKey = (ContextLocal<T>) key;
                 return (T) context.getLocal(localKey, QuarkusAccessModes.ACQUIRE_RELEASE);
             }
+            assert !(key instanceof ContextLocal<?>);
             return (T) context.localContextData().get(key);
         } else {
             throw new UnsupportedOperationException(ILLEGAL_ACCESS_TO_LOCAL_CONTEXT);
@@ -37,10 +38,11 @@ public static <T> T getLocal(ContextInternal context, Object key) {
     public static void putLocal(ContextInternal context, Object key, Object value) {
         if (VertxContext.isDuplicatedContext(context)) {
             // We are on a duplicated context, allow accessing the locals
-            if (key instanceof ContextLocalImpl<?> || key instanceof ContextLocal<?>) {
+            if (key instanceof ContextLocalImpl<?>) {
                 var localKey = (ContextLocal<Object>) key;
                 context.putLocal(localKey, QuarkusAccessModes.ACQUIRE_RELEASE, value);
             } else {
+                assert !(key instanceof ContextLocal<?>);
                 context.localContextData().put(key, value);
             }
         } else {
@@ -51,14 +53,15 @@ public static void putLocal(ContextInternal context, Object key, Object value) {
     public static boolean removeLocal(ContextInternal context, Object key) {
         if (VertxContext.isDuplicatedContext(context)) {
             // We are on a duplicated context, allow accessing the locals
-            if (key instanceof ContextLocalImpl<?> || key instanceof ContextLocal<?>) {
+            if (key instanceof ContextLocalImpl<?>) {
                 var localKey = (ContextLocal<Object>) key;
                 if (localKey == null) {
                     return false;
                 }
                 context.removeLocal(localKey, QuarkusAccessModes.ACQUIRE_RELEASE);
                 return true;
             }
+            assert !(key instanceof ContextLocal<?>);
             return context.localContextData().remove(key) != null;
         } else {
             throw new UnsupportedOperationException(ILLEGAL_ACCESS_TO_LOCAL_CONTEXT);

diff --git a/...runtime/src/main/java/io/quarkus/vertx/core/runtime/context/VertxContextSafetyToggle.java b/...runtime/src/main/java/io/quarkus/vertx/core/runtime/context/VertxContextSafetyToggle.java
@@ -1,9 +1,11 @@
 package io.quarkus.vertx.core.runtime.context;
 
+import static io.quarkus.vertx.runtime.storage.QuarkusLocalStorageKeyVertxServiceProvider.ACCESS_TOGGLE_KEY;
+
 import io.smallrye.common.vertx.VertxContext;
 import io.vertx.core.Context;
 import io.vertx.core.Vertx;
-import io.vertx.core.impl.ContextLocalImpl;
+import io.vertx.core.spi.context.storage.ContextLocal;
 
 /**
  * This is meant for other extensions to integrate with, to help
@@ -49,9 +51,12 @@ public final class VertxContextSafetyToggle {
     public static final String FULLY_DISABLE_PROPERTY = "io.quarkus.vertx.core.runtime.context.VertxContextSafetyToggle.I_HAVE_CHECKED_EVERYTHING";
     private static final boolean UNRESTRICTED_BY_DEFAULT = Boolean.getBoolean(UNRESTRICTED_BY_DEFAULT_PROPERTY);
     private static final boolean FULLY_DISABLED = Boolean.getBoolean(FULLY_DISABLE_PROPERTY);
-    // TODO VertxImpl should be allocated AFTER getting here, to make it work!
-    // ContextLocalImpl permanently assign a globally unique key: if the check is disabled, there's no point in creating it
-    private static final ContextLocalImpl<Boolean> ACCESS_TOGGLE_KEY = FULLY_DISABLED ? new ContextLocalImpl<>() : null;
+
+    public static ContextLocal<Boolean> registerAccessToggleKey() {
+        if (FULLY_DISABLED)
+            return null;
+        return ContextLocal.registerLocal(Boolean.class);
+    }
 
     /**
      * Verifies if the current Vert.x context was flagged as safe

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
@@ -1,5 +1,7 @@
 package io.quarkus.vertx.runtime;
 
+import static io.quarkus.vertx.runtime.storage.QuarkusLocalStorageKeyVertxServiceProvider.REQUEST_SCOPED_LOCAL_KEY;
+
 import java.lang.annotation.Annotation;
 import java.util.List;
 import java.util.concurrent.ConcurrentMap;
@@ -18,13 +20,10 @@
 import io.vertx.core.Context;
 import io.vertx.core.Vertx;
 import io.vertx.core.impl.ContextInternal;
-import io.vertx.core.impl.ContextLocalImpl;
 
 public class VertxCurrentContextFactory implements CurrentContextFactory {
 
     private static final String LOCAL_KEY_PREFIX = "io.quarkus.vertx.cdi-current-context";
-    // TODO VertxImpl should be allocated AFTER getting here, to make it work!
-    private static final ContextLocalImpl<? extends ContextState> REQUEST_SCOPED_LOCAL_KEY = new ContextLocalImpl<>();
 
     private final List<String> keys;
     private final AtomicBoolean requestScopedKeyCreated;
@@ -38,7 +37,7 @@ public VertxCurrentContextFactory() {
     @Override
     public <T extends InjectableContext.ContextState> CurrentContext<T> create(Class<? extends Annotation> scope) {
         if (scope == RequestScoped.class) {
-            if (!requestScopedKeyCreated.compareAndExchange(false, true)) {
+            if (!requestScopedKeyCreated.compareAndSet(false, true)) {
                 throw new IllegalStateException(
                         "Multiple current contexts for the same scope are not supported. Current context for "
                                 + scope + " already exists!");

diff --git a/...ain/java/io/quarkus/vertx/runtime/storage/QuarkusLocalStorageKeyVertxServiceProvider.java b/...ain/java/io/quarkus/vertx/runtime/storage/QuarkusLocalStorageKeyVertxServiceProvider.java
@@ -0,0 +1,23 @@
+package io.quarkus.vertx.runtime.storage;
+
+import io.quarkus.arc.InjectableContext;
+import io.quarkus.vertx.core.runtime.context.VertxContextSafetyToggle;
+import io.vertx.core.impl.VertxBuilder;
+import io.vertx.core.spi.VertxServiceProvider;
+import io.vertx.core.spi.context.storage.ContextLocal;
+
+/**
+ * This provider exists with the sole purpose of reliably get the optimized local keys created before
+ * the Vertx instance is created.
+ */
+public class QuarkusLocalStorageKeyVertxServiceProvider implements VertxServiceProvider {
+
+    public static final ContextLocal<Boolean> ACCESS_TOGGLE_KEY = VertxContextSafetyToggle.registerAccessToggleKey();
+    public static final ContextLocal<InjectableContext.ContextState> REQUEST_SCOPED_LOCAL_KEY = ContextLocal
+            .registerLocal(InjectableContext.ContextState.class);
+
+    @Override
+    public void init(VertxBuilder builder) {
+
+    }
+}
diff --git a/...vertx/runtime/src/main/resources/META-INF/services/io.vertx.core.spi.VertxServiceProvider b/...vertx/runtime/src/main/resources/META-INF/services/io.vertx.core.spi.VertxServiceProvider
@@ -0,0 +1 @@
+io.quarkus.vertx.runtime.storage.QuarkusLocalStorageKeyVertxServiceProvider