NPE in NodeImpl (#1153)

* attempt to fix NPE in NodeImpl * throw IllegalStateException in LogManagerImpl
sofastack · Sep 28, 2024 · 99d6afe · 99d6afe
1 parent 494f868
commit 99d6afe
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 0 deletions.
diff --git a/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/impl/LogManagerImpl.java b/jraft-core/src/main/java/com/alipay/sofa/jraft/storage/impl/LogManagerImpl.java
@@ -846,6 +846,11 @@ public long getLastLogIndex(final boolean isFlush) {
             Thread.currentThread().interrupt();
             throw new IllegalStateException(e);
         }
+        if (c.lastLogId == null) {
+            assert stopped : "Last log id can be null only when node is stopping.";
+
+            throw new IllegalStateException("Node is shutting down");
+        }
         return c.lastLogId.getIndex();
     }
 
@@ -894,6 +899,11 @@ public LogId getLastLogId(final boolean isFlush) {
             Thread.currentThread().interrupt();
             throw new IllegalStateException(e);
         }
+        if (c.lastLogId == null) {
+            assert stopped : "Last log id can be null only when node is stopping.";
+
+            throw new IllegalStateException("Node is shutting down");
+        }
         return c.lastLogId;
     }
 

diff --git a/jraft-core/src/test/java/com/alipay/sofa/jraft/storage/impl/LogManagerTest.java b/jraft-core/src/test/java/com/alipay/sofa/jraft/storage/impl/LogManagerTest.java
@@ -51,6 +51,7 @@
 import static org.junit.Assert.assertNotSame;
 import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 
 @RunWith(value = MockitoJUnitRunner.class)
@@ -416,4 +417,23 @@ public void testCheckAndSetConfiguration() throws Exception {
         assertEquals("localhost:8081,localhost:8082", lastEntry.getOldConf().toString());
     }
 
+    @Test
+    public void testLastLogIdWhenShutdown() throws Exception {
+        mockAddEntries();
+        assertEquals(1, this.logManager.getFirstLogIndex());
+        assertEquals(10, this.logManager.getLastLogIndex());
+        this.logManager.shutdown();
+        Exception e = assertThrows(IllegalStateException.class, () -> this.logManager.getLastLogId(true));
+        assertEquals("Node is shutting down", e.getMessage());
+    }
+
+    @Test
+    public void testLastLogIndexWhenShutdown() throws Exception {
+        mockAddEntries();
+        assertEquals(1, this.logManager.getFirstLogIndex());
+        assertEquals(10, this.logManager.getLastLogIndex());
+        this.logManager.shutdown();
+        Exception e = assertThrows(IllegalStateException.class, () -> this.logManager.getLastLogIndex(true));
+        assertEquals("Node is shutting down", e.getMessage());
+    }
 }