gnodet-bot commented on code in PR #29681:
URL: https://github.com/apache/camel/pull/29681#discussion_r4551222164


##########
components/camel-vertx/camel-vertx/src/main/java/org/apache/camel/component/vertx/VertxComponent.java:
##########
@@ -162,34 +164,62 @@ protected void doStart() throws Exception {
             createdVertx = true;
 
             final CountDownLatch latch = new CountDownLatch(1);
+            final AtomicReference<Throwable> failure = new AtomicReference<>();
+            // the callback writes here rather than to the field, so a build 
that completes after the timeout
+            // cannot leave a half-published Vert.x behind
+            final AtomicReference<Vertx> created = new AtomicReference<>();
+            Future<Vertx> clusteredVertx = null;
 
             // lets using a host / port if a host name is specified
             if (clustered) {
+                if (timeout <= 0) {
+                    throw new IllegalArgumentException(
+                            "The timeout option must be a positive number of 
seconds, was: " + timeout);
+                }
+
                 LOG.info("Creating Clustered Vertx {}:{}", 
vertxOptions.getEventBusOptions().getHost(),
                         vertxOptions.getEventBusOptions().getPort());
                 // use the async api as we want to wait for the eventbus to be 
ready before we are in started state
-                vertxFactory.buildClustered().onComplete(event -> {
+                clusteredVertx = vertxFactory.buildClustered();
+                clusteredVertx.onComplete(event -> {
                     if (event.cause() != null) {
-                        LOG.warn("Error creating Clustered Vertx {}:{} due 
{}", host, port,
-                                event.cause().getMessage(), event.cause());
+                        failure.set(event.cause());
                     } else if (event.succeeded()) {
-                        vertx = event.result();
-                        LOG.info("EventBus is ready: {}", vertx);
+                        created.set(event.result());
                     }
 
                     latch.countDown();
                 });
             } else {
                 LOG.info("Creating Non-Clustered Vertx");
-                vertx = vertxFactory.build();
-                LOG.info("EventBus is ready: {}", vertx);
+                created.set(vertxFactory.build());
                 latch.countDown();
             }
 
             if (latch.getCount() > 0) {
                 LOG.info("Waiting for EventBus to be ready using {} sec as 
timeout", timeout);
-                latch.await(timeout, TimeUnit.SECONDS);
+                if (!latch.await(timeout, TimeUnit.SECONDS)) {
+                    // the build is still running: a node that joins the 
cluster after we gave up would keep its
+                    // event-loop threads alive with nobody left to close it. 
onSuccess also fires at once if the
+                    // future completed between the await and this 
registration, so no window is left open
+                    clusteredVertx.onSuccess(Vertx::close);

Review Comment:
   The late-arriving Vert.x is closed silently here. In production, if a node 
completes the cluster join after the timeout, operators may see Vert.x 
lifecycle events without any log explaining why the instance was discarded.
   
   Consider:
   ```suggestion
                       clusteredVertx.onSuccess(v -> {
                           LOG.warn("A Vert.x instance arrived after the 
timeout — closing it to prevent resource leak");
                           v.close();
                       });
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to