(ratis) branch master updated: RATIS-2521. LifeCycle.startAndTransition(..) may cause illegal transition: CLOSING -> EXCEPTION. (#1452)

[szetszwo]

This is an automated email from the ASF dual-hosted git repository.

szetszwo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ratis.git


The following commit(s) were added to refs/heads/master by this push:
     new 5db7830c0 RATIS-2521. LifeCycle.startAndTransition(..) may cause 
illegal transition: CLOSING -> EXCEPTION. (#1452)
5db7830c0 is described below

commit 5db7830c01521a84a8cd4740f953c0b0132016ba
Author: Tsz-Wo Nicholas Sze <szets...@apache.org>
AuthorDate: Fri May 8 09:14:26 2026 -0700

    RATIS-2521. LifeCycle.startAndTransition(..) may cause illegal transition: 
CLOSING -> EXCEPTION. (#1452)
---
 .../main/java/org/apache/ratis/util/LifeCycle.java | 11 ++--
 .../java/org/apache/ratis/util/TestLifeCycle.java  | 69 +++++++++++++++++++++-
 2 files changed, 75 insertions(+), 5 deletions(-)

diff --git a/ratis-common/src/main/java/org/apache/ratis/util/LifeCycle.java 
b/ratis-common/src/main/java/org/apache/ratis/util/LifeCycle.java
index e96ba88a5..86846b81d 100644
--- a/ratis-common/src/main/java/org/apache/ratis/util/LifeCycle.java
+++ b/ratis-common/src/main/java/org/apache/ratis/util/LifeCycle.java
@@ -273,15 +273,18 @@ public class LifeCycle {
   /** Run the given start method and transition the current state accordingly. 
*/
   @SafeVarargs
   public final <T extends Throwable> void startAndTransition(
-      CheckedRunnable<T> startImpl, Class<? extends Throwable>... 
exceptionClasses)
+      CheckedRunnable<T> startMethod, Class<? extends Throwable>... 
exceptionClasses)
       throws T {
     transition(State.STARTING);
     try {
-      startImpl.run();
+      startMethod.run();
       transition(State.RUNNING);
     } catch (Throwable t) {
-      transition(ReflectionUtils.isInstance(t, exceptionClasses)?
-          State.NEW: State.EXCEPTION);
+      final State state = getCurrentState();
+      LOG.warn("{}: Failed to start (state={})", name, state, t);
+      if (!state.isClosingOrClosed()) {
+        transition(ReflectionUtils.isInstance(t, exceptionClasses) ? State.NEW 
: State.EXCEPTION);
+      }
       throw t;
     }
   }
diff --git a/ratis-test/src/test/java/org/apache/ratis/util/TestLifeCycle.java 
b/ratis-test/src/test/java/org/apache/ratis/util/TestLifeCycle.java
index 201b51057..92a6e3c41 100644
--- a/ratis-test/src/test/java/org/apache/ratis/util/TestLifeCycle.java
+++ b/ratis-test/src/test/java/org/apache/ratis/util/TestLifeCycle.java
@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -25,10 +25,13 @@ import java.util.Arrays;
 import java.util.EnumMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
 
 import static org.apache.ratis.util.LifeCycle.State.*;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 
@@ -101,4 +104,68 @@ public class TestLifeCycle {
     }
   }
 
+  @Test
+  public void testStartAndTransition() throws Exception {
+    final SimulatedServer simulatedServer = new SimulatedServer();
+    assertEquals(NEW, simulatedServer.getLifeCycleState());
+
+    final CompletableFuture<Throwable> f = CompletableFuture.supplyAsync(() -> 
{
+      try {
+        simulatedServer.start();
+        throw new AssertionError("start() should fail");
+      } catch (Exception e) {
+        return e.getCause();
+      }
+    });
+
+    Thread.sleep(100);
+    assertEquals(STARTING, simulatedServer.getLifeCycleState());
+
+    // call close() during STARTING, start() should throw the simulated 
exception
+    CompletableFuture.supplyAsync(simulatedServer::close);
+    assertSame(simulatedServer.getSimulatedException(), f.get());
+
+    assertEquals(CLOSING, simulatedServer.getLifeCycleState());
+    simulatedServer.getCloseFuture().complete(null);
+    Thread.sleep(100);
+    assertEquals(CLOSED, simulatedServer.getLifeCycleState());
+  }
+
+  private static final class SimulatedServer {
+    private final LifeCycle lifeCycle = new 
LifeCycle(getClass().getSimpleName());
+    private final Exception simulatedException = new Exception("Simulated 
exception");
+    private final CompletableFuture<Void> startFuture = new 
CompletableFuture<>();
+    private final CompletableFuture<Void> closeFuture = new 
CompletableFuture<>();
+
+    LifeCycle.State getLifeCycleState() {
+      return lifeCycle.getCurrentState();
+    }
+
+    Exception getSimulatedException() {
+      return simulatedException;
+    }
+
+    CompletableFuture<Void> getCloseFuture() {
+      return closeFuture;
+    }
+
+    void start() throws Exception {
+      lifeCycle.startAndTransition(this::startImpl);
+    }
+
+    void startImpl() throws Exception {
+      startFuture.get();
+    }
+
+    Void close() {
+      // simulate close and then cause start() to fail.
+      lifeCycle.checkStateAndClose(this::closeImpl);
+      return null;
+    }
+
+    void closeImpl() {
+      startFuture.completeExceptionally(simulatedException);
+      closeFuture.join();
+    }
+  }
 }