From 9bfa6d47e326cfc3138ce1fe9a385070c78e3260 Mon Sep 17 00:00:00 2001
From: Julien Viet <julien@julienviet.com>
Date: Thu, 21 Sep 2023 11:45:42 +0200
Subject: [PATCH] The recent changes in worker pool affected worker verticles
 with a regression: the named verticle worker pool is closed before
 undeploying the verticle which leads to have the worker verticle undeployment
 fail when the worker pool is used only by this verticle.

Close the worker pool after the verticle has been deployed instead of before.
---
 .../io/vertx/core/impl/DeploymentManager.java |  6 ++---
 .../io/vertx/core/NamedWorkerPoolTest.java    | 24 ++++++++++++++++++-
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/main/java/io/vertx/core/impl/DeploymentManager.java b/src/main/java/io/vertx/core/impl/DeploymentManager.java
index cf487320f54..31e8f6bfdd4 100644
--- a/src/main/java/io/vertx/core/impl/DeploymentManager.java
+++ b/src/main/java/io/vertx/core/impl/DeploymentManager.java
@@ -306,9 +306,6 @@ public synchronized Future<Void> doUndeploy(ContextInternal undeployingContext)
         status = ST_UNDEPLOYING;
         return doUndeployChildren(undeployingContext).compose(v -> doUndeploy(undeployingContext));
       } else {
-        if (workerPool != null) {
-          workerPool.close();
-        }
         status = ST_UNDEPLOYED;
         List<Future<?>> undeployFutures = new ArrayList<>();
         if (parent != null) {
@@ -340,6 +337,9 @@ public synchronized Future<Void> doUndeploy(ContextInternal undeployingContext)
         Promise<Void> resolvingPromise = undeployingContext.promise();
         Future.all(undeployFutures).<Void>mapEmpty().onComplete(resolvingPromise);
         Future<Void> fut = resolvingPromise.future();
+        if (workerPool != null) {
+          fut = fut.andThen(ar -> workerPool.close());
+        }
         Handler<Void> handler = undeployHandler;
         if (handler != null) {
           undeployHandler = null;
diff --git a/src/test/java/io/vertx/core/NamedWorkerPoolTest.java b/src/test/java/io/vertx/core/NamedWorkerPoolTest.java
index 4b3cea7c8af..e028f3b4d3e 100644
--- a/src/test/java/io/vertx/core/NamedWorkerPoolTest.java
+++ b/src/test/java/io/vertx/core/NamedWorkerPoolTest.java
@@ -22,6 +22,7 @@
 import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 
 import static java.util.concurrent.TimeUnit.*;
@@ -268,7 +269,7 @@ public void start() throws Exception {
   }
 
   @Test
-  public void testDeployUsingNamedPool() throws Exception {
+  public void testDeployUsingNamedPool() {
     AtomicReference<Thread> thread = new AtomicReference<>();
     String poolName = "vert.x-" + TestUtils.randomAlphaString(10);
     Promise<Void> undeployed = Promise.promise();
@@ -292,6 +293,27 @@ public void start() {
     assertWaitUntil(() -> thread.get() != null && thread.get().getState() == Thread.State.TERMINATED);
   }
 
+  @Test
+  public void testNamedWorkerPoolShouldBeClosedAfterVerticleIsUndeployed() {
+    AtomicReference<String> threadName = new AtomicReference<>();
+    vertx.deployVerticle(new AbstractVerticle() {
+      @Override
+      public void start() {
+      }
+      @Override
+      public void stop() {
+        threadName.set(Thread.currentThread().getName());
+      }
+    }, new DeploymentOptions().setWorker(true).setWorkerPoolName("test-worker")).onComplete(onSuccess(id -> {
+      vertx.undeploy(id).onComplete(onSuccess(v -> {
+        assertNotNull(threadName.get());
+        assertTrue(threadName.get().startsWith("test-worker"));
+        testComplete();
+      }));
+    }));
+    await();
+  }
+
   @Test
   public void testDeployUsingNamedWorkerDoesNotCreateExtraEventLoop() {
     int instances = getOptions().getEventLoopPoolSize();