Repository: eclipse-vertx/vert.x

Problem statement:
Remove legacy TCCL check log
This log is not really helpful, in addition it is possible to disable TCCL and have a TCCL set to the non system classloader.


Hints:

Patch:
diff --git a/src/main/generated/io/vertx/core/VertxOptionsConverter.java b/src/main/generated/io/vertx/core/VertxOptionsConverter.java
--- a/src/main/generated/io/vertx/core/VertxOptionsConverter.java
+++ b/src/main/generated/io/vertx/core/VertxOptionsConverter.java
@@ -31,6 +31,11 @@ static void fromJson(Iterable<java.util.Map.Entry<String, Object>> json, VertxOp
             obj.setBlockedThreadCheckIntervalUnit(java.util.concurrent.TimeUnit.valueOf((String)member.getValue()));
           }
           break;
+        case "disableTCCL":
+          if (member.getValue() instanceof Boolean) {
+            obj.setDisableTCCL((Boolean)member.getValue());
+          }
+          break;
         case "eventBusOptions":
           if (member.getValue() instanceof JsonObject) {
             obj.setEventBusOptions(new io.vertx.core.eventbus.EventBusOptions((io.vertx.core.json.JsonObject)member.getValue()));
@@ -132,6 +137,7 @@ static void toJson(VertxOptions obj, java.util.Map<String, Object> json) {
     if (obj.getBlockedThreadCheckIntervalUnit() != null) {
       json.put("blockedThreadCheckIntervalUnit", obj.getBlockedThreadCheckIntervalUnit().name());
     }
+    json.put("disableTCCL", obj.getDisableTCCL());
     if (obj.getEventBusOptions() != null) {
       json.put("eventBusOptions", obj.getEventBusOptions().toJson());
     }
diff --git a/src/main/java/io/vertx/core/VertxOptions.java b/src/main/java/io/vertx/core/VertxOptions.java
--- a/src/main/java/io/vertx/core/VertxOptions.java
+++ b/src/main/java/io/vertx/core/VertxOptions.java
@@ -32,6 +32,8 @@
 @DataObject(generateConverter = true, publicConverter = false)
 public class VertxOptions {
 
+  private static final String DISABLE_TCCL_PROP_NAME = "vertx.disableTCCL";
+
   /**
    * The default number of event loop threads to be used  = 2 * number of cores on the machine
    */
@@ -109,6 +111,8 @@ public class VertxOptions {
    */
   public static final TimeUnit DEFAULT_WARNING_EXCEPTION_TIME_UNIT = TimeUnit.NANOSECONDS;
 
+  public static final boolean DEFAULT_DISABLE_TCCL = Boolean.getBoolean(DISABLE_TCCL_PROP_NAME);
+
   private int eventLoopPoolSize = DEFAULT_EVENT_LOOP_POOL_SIZE;
   private int workerPoolSize = DEFAULT_WORKER_POOL_SIZE;
   private int internalBlockingPoolSize = DEFAULT_INTERNAL_BLOCKING_POOL_SIZE;
@@ -130,6 +134,7 @@ public class VertxOptions {
   private TimeUnit maxWorkerExecuteTimeUnit = DEFAULT_MAX_WORKER_EXECUTE_TIME_UNIT;
   private TimeUnit warningExceptionTimeUnit = DEFAULT_WARNING_EXCEPTION_TIME_UNIT;
   private TimeUnit blockedThreadCheckIntervalUnit = DEFAULT_BLOCKED_THREAD_CHECK_INTERVAL_UNIT;
+  private boolean disableTCCL = DEFAULT_DISABLE_TCCL;
 
   /**
    * Default constructor
@@ -163,6 +168,7 @@ public VertxOptions(VertxOptions other) {
     this.warningExceptionTimeUnit = other.warningExceptionTimeUnit;
     this.blockedThreadCheckIntervalUnit = other.blockedThreadCheckIntervalUnit;
     this.tracingOptions = other.tracingOptions != null ? other.tracingOptions.copy() : null;
+    this.disableTCCL = other.disableTCCL;
   }
 
   /**
@@ -643,6 +649,32 @@ public VertxOptions setTracingOptions(TracingOptions tracingOptions) {
     return this;
   }
 
+  /**
+   * @return whether Vert.x sets the {@link Context} classloader as the thread context classloader on actions executed on that {@link Context}
+   */
+  public boolean getDisableTCCL() {
+    return disableTCCL;
+  }
+
+  /**
+   * Configures whether Vert.x sets the {@link Context} classloader as the thread context classloader on actions executed on that {@link Context}.
+   *
+   * When a {@link Context} is created the current thread classloader is captured and associated with this classloader.
+   *
+   * Likewise when a Verticle is created, the Verticle's {@link Context} classloader is set to the current thread classloader
+   * unless this classloader is overriden by {@link DeploymentOptions#getClassLoader()}.
+   *
+   * This setting overrides the (legacy) system property {@code vertx.disableTCCL} and provides control at the
+   * Vertx instance level.
+   *
+   * @param disableTCCL {@code true} to disable thread context classloader update by Vertx
+   * @return a reference to this, so the API can be used fluently
+   */
+  public VertxOptions setDisableTCCL(boolean disableTCCL) {
+    this.disableTCCL = disableTCCL;
+    return this;
+  }
+
   public JsonObject toJson() {
     JsonObject json = new JsonObject();
     VertxOptionsConverter.toJson(this, json);
@@ -673,6 +705,7 @@ public String toString() {
         ", eventbus=" + eventBusOptions.toJson() +
         ", warningExceptionTimeUnit=" + warningExceptionTimeUnit +
         ", warningExceptionTime=" + warningExceptionTime +
+        ", disableTCCL=" + disableTCCL +
         '}';
   }
 }
diff --git a/src/main/java/io/vertx/core/impl/AbstractContext.java b/src/main/java/io/vertx/core/impl/AbstractContext.java
--- a/src/main/java/io/vertx/core/impl/AbstractContext.java
+++ b/src/main/java/io/vertx/core/impl/AbstractContext.java
@@ -19,8 +19,6 @@
 
 import java.util.List;
 
-import static io.vertx.core.impl.VertxThread.DISABLE_TCCL;
-
 /**
  * A context implementation that does not hold any specific state.
  *
@@ -29,6 +27,12 @@
  */
 abstract class AbstractContext implements ContextInternal {
 
+  final boolean disableTCCL;
+
+  public AbstractContext(boolean disableTCCL) {
+    this.disableTCCL = disableTCCL;
+  }
+
   @Override
   public abstract boolean isEventLoopContext();
 
@@ -63,7 +67,7 @@ public final ContextInternal beginDispatch() {
     ContextInternal prev;
     VertxThread th = (VertxThread) Thread.currentThread();
     prev = th.beginEmission(this);
-    if (!DISABLE_TCCL) {
+    if (!disableTCCL) {
       th.setContextClassLoader(classLoader());
     }
     return prev;
@@ -71,7 +75,7 @@ public final ContextInternal beginDispatch() {
 
   public final void endDispatch(ContextInternal previous) {
     VertxThread th = (VertxThread) Thread.currentThread();
-    if (!DISABLE_TCCL) {
+    if (!disableTCCL) {
       th.setContextClassLoader(previous != null ? previous.classLoader() : null);
     }
     th.endEmission(previous);
diff --git a/src/main/java/io/vertx/core/impl/ContextImpl.java b/src/main/java/io/vertx/core/impl/ContextImpl.java
--- a/src/main/java/io/vertx/core/impl/ContextImpl.java
+++ b/src/main/java/io/vertx/core/impl/ContextImpl.java
@@ -75,10 +75,9 @@ static void executeIsolated(Handler<Void> task) {
               WorkerPool workerPool,
               Deployment deployment,
               CloseFuture closeFuture,
-              ClassLoader tccl) {
-    if (VertxThread.DISABLE_TCCL && tccl != ClassLoader.getSystemClassLoader()) {
-      log.warn("You have disabled TCCL checks but you have a custom TCCL to set.");
-    }
+              ClassLoader tccl,
+              boolean disableTCCL) {
+    super(disableTCCL);
     this.deployment = deployment;
     this.config = deployment != null ? deployment.config() : new JsonObject();
     this.eventLoop = eventLoop;
diff --git a/src/main/java/io/vertx/core/impl/DuplicatedContext.java b/src/main/java/io/vertx/core/impl/DuplicatedContext.java
--- a/src/main/java/io/vertx/core/impl/DuplicatedContext.java
+++ b/src/main/java/io/vertx/core/impl/DuplicatedContext.java
@@ -38,6 +38,7 @@ class DuplicatedContext extends AbstractContext {
   private ConcurrentMap<Object, Object> localData;
 
   DuplicatedContext(ContextImpl delegate) {
+    super(delegate.disableTCCL);
     this.delegate = delegate;
   }
 
diff --git a/src/main/java/io/vertx/core/impl/EventLoopContext.java b/src/main/java/io/vertx/core/impl/EventLoopContext.java
--- a/src/main/java/io/vertx/core/impl/EventLoopContext.java
+++ b/src/main/java/io/vertx/core/impl/EventLoopContext.java
@@ -27,8 +27,9 @@ public class EventLoopContext extends ContextImpl {
                    WorkerPool workerPool,
                    Deployment deployment,
                    CloseFuture closeFuture,
-                   ClassLoader tccl) {
-    super(vertx, eventLoop, internalBlockingPool, workerPool, deployment, closeFuture, tccl);
+                   ClassLoader tccl,
+                   boolean disableTCCL) {
+    super(vertx, eventLoop, internalBlockingPool, workerPool, deployment, closeFuture, tccl, disableTCCL);
   }
 
   @Override
diff --git a/src/main/java/io/vertx/core/impl/VertxImpl.java b/src/main/java/io/vertx/core/impl/VertxImpl.java
--- a/src/main/java/io/vertx/core/impl/VertxImpl.java
+++ b/src/main/java/io/vertx/core/impl/VertxImpl.java
@@ -133,6 +133,7 @@ public class VertxImpl implements VertxInternal, MetricsProvider {
   private final Transport transport;
   private final VertxTracer tracer;
   private final ThreadLocal<WeakReference<AbstractContext>> stickyContext = new ThreadLocal<>();
+  private final boolean disableTCCL;
 
   VertxImpl(VertxOptions options, ClusterManager clusterManager, NodeSelector nodeSelector, VertxMetrics metrics,
             VertxTracer<?, ?> tracer, Transport transport, FileResolver fileResolver, VertxThreadFactory threadFactory,
@@ -165,6 +166,7 @@ public class VertxImpl implements VertxInternal, MetricsProvider {
     defaultWorkerPoolSize = options.getWorkerPoolSize();
     maxWorkerExecTime = options.getMaxWorkerExecuteTime();
     maxWorkerExecTimeUnit = options.getMaxWorkerExecuteTimeUnit();
+    disableTCCL = options.getDisableTCCL();
 
     this.executorServiceFactory = executorServiceFactory;
     this.threadFactory = threadFactory;
@@ -471,12 +473,12 @@ public boolean cancelTimer(long id) {
 
   @Override
   public EventLoopContext createEventLoopContext(Deployment deployment, CloseFuture closeFuture, WorkerPool workerPool, ClassLoader tccl) {
-    return new EventLoopContext(this, eventLoopGroup.next(), internalWorkerPool, workerPool != null ? workerPool : this.workerPool, deployment, closeFuture, tccl);
+    return new EventLoopContext(this, eventLoopGroup.next(), internalWorkerPool, workerPool != null ? workerPool : this.workerPool, deployment, closeFuture, tccl, disableTCCL);
   }
 
   @Override
   public EventLoopContext createEventLoopContext(EventLoop eventLoop, WorkerPool workerPool, ClassLoader tccl) {
-    return new EventLoopContext(this, eventLoop, internalWorkerPool, workerPool != null ? workerPool : this.workerPool, null, closeFuture, tccl);
+    return new EventLoopContext(this, eventLoop, internalWorkerPool, workerPool != null ? workerPool : this.workerPool, null, closeFuture, tccl, disableTCCL);
   }
 
   @Override
@@ -486,7 +488,7 @@ public EventLoopContext createEventLoopContext() {
 
   @Override
   public WorkerContext createWorkerContext(Deployment deployment, CloseFuture closeFuture, WorkerPool workerPool, ClassLoader tccl) {
-    return new WorkerContext(this, internalWorkerPool, workerPool != null ? workerPool : this.workerPool, deployment, closeFuture, tccl);
+    return new WorkerContext(this, internalWorkerPool, workerPool != null ? workerPool : this.workerPool, deployment, closeFuture, tccl, disableTCCL);
   }
 
   @Override
diff --git a/src/main/java/io/vertx/core/impl/VertxThread.java b/src/main/java/io/vertx/core/impl/VertxThread.java
--- a/src/main/java/io/vertx/core/impl/VertxThread.java
+++ b/src/main/java/io/vertx/core/impl/VertxThread.java
@@ -20,9 +20,6 @@
  */
 public class VertxThread extends FastThreadLocalThread implements BlockedThreadChecker.Task {
 
-  static final String DISABLE_TCCL_PROP_NAME = "vertx.disableTCCL";
-  static final boolean DISABLE_TCCL = Boolean.getBoolean(DISABLE_TCCL_PROP_NAME);
-
   private final boolean worker;
   private final long maxExecTime;
   private final TimeUnit maxExecTimeUnit;
diff --git a/src/main/java/io/vertx/core/impl/WorkerContext.java b/src/main/java/io/vertx/core/impl/WorkerContext.java
--- a/src/main/java/io/vertx/core/impl/WorkerContext.java
+++ b/src/main/java/io/vertx/core/impl/WorkerContext.java
@@ -28,8 +28,9 @@ public class WorkerContext extends ContextImpl {
                 WorkerPool workerPool,
                 Deployment deployment,
                 CloseFuture closeFuture,
-                ClassLoader tccl) {
-    super(vertx, vertx.getEventLoopGroup().next(), internalBlockingPool, workerPool, deployment, closeFuture, tccl);
+                ClassLoader tccl,
+                boolean disableTCCL) {
+    super(vertx, vertx.getEventLoopGroup().next(), internalBlockingPool, workerPool, deployment, closeFuture, tccl, disableTCCL);
   }
 
   @Override


Test patch:
diff --git a/src/test/benchmarks/io/vertx/core/impl/BenchmarkContext.java b/src/test/benchmarks/io/vertx/core/impl/BenchmarkContext.java
--- a/src/test/benchmarks/io/vertx/core/impl/BenchmarkContext.java
+++ b/src/test/benchmarks/io/vertx/core/impl/BenchmarkContext.java
@@ -30,7 +30,7 @@ public static BenchmarkContext create(Vertx vertx) {
   }
 
   public BenchmarkContext(VertxInternal vertx, WorkerPool internalBlockingPool, WorkerPool workerPool, ClassLoader tccl) {
-    super(vertx, vertx.getEventLoopGroup().next(), internalBlockingPool, workerPool, null, null, tccl);
+    super(vertx, vertx.getEventLoopGroup().next(), internalBlockingPool, workerPool, null, null, tccl, false);
   }
 
   @Override
diff --git a/src/test/java/io/vertx/core/VertxTest.java b/src/test/java/io/vertx/core/VertxTest.java
--- a/src/test/java/io/vertx/core/VertxTest.java
+++ b/src/test/java/io/vertx/core/VertxTest.java
@@ -26,6 +26,8 @@
 import org.openjdk.jmh.runner.options.OptionsBuilder;
 
 import java.lang.ref.WeakReference;
+import java.net.URL;
+import java.net.URLClassLoader;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -290,4 +292,30 @@ public void testCloseVertxShouldWaitConcurrentCloseHook() throws Exception {
     ref.get().complete();
     assertWaitUntil(closed::get);
   }
+
+  @Test
+  public void testEnableTCCL() {
+    testTCCL(false);
+  }
+
+  @Test
+  public void testDisableTCCL() {
+    testTCCL(true);
+  }
+
+  private void testTCCL(boolean disable) {
+    VertxOptions options = new VertxOptions().setDisableTCCL(disable);
+    Vertx vertx = Vertx.vertx(options);
+    ClassLoader orig = Thread.currentThread().getContextClassLoader();
+    ClassLoader cl = new URLClassLoader(new URL[0], orig);
+    Thread.currentThread().setContextClassLoader(cl);
+    Context ctx = vertx.getOrCreateContext();
+    Thread.currentThread().setContextClassLoader(orig);
+    ctx.runOnContext(v -> {
+      ClassLoader expected = disable ? orig : cl;
+      assertSame(expected, Thread.currentThread().getContextClassLoader());
+      testComplete();
+    });
+    await();
+  }
 }