From 791414abed1bf0576b0bd6124d16715624251400 Mon Sep 17 00:00:00 2001 From: gaozhangmin Date: Wed, 11 Aug 2021 09:18:10 +0800 Subject: [PATCH] fix getPreviousPosition npe (#11621) Fixes #11619 ### Motivation There's a chance that headMap might be empty even if a empty check added. because remove would be executed in ledgers. after the empty check between line 3214 and line 3219 in following code snippet: headMap is a view of ledgers pulsar/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java Lines 3206 to 3222 in 31231d6 ``` NavigableMap headMap = ledgers.headMap(position.getLedgerId(), false); if (headMap.isEmpty()) { // There is no previous ledger, return an invalid position in the current ledger return PositionImpl.get(position.getLedgerId(), -1); } // We need to find the most recent non-empty ledger for (long ledgerId : headMap.descendingKeySet()) { LedgerInfo li = headMap.get(ledgerId); if (li.getEntries() > 0) { return PositionImpl.get(li.getLedgerId(), li.getEntries() - 1); } } // in case there are only empty ledgers, we return a position in the first one return PositionImpl.get(headMap.firstEntry().getKey(), -1); ``` ### Modifications Add a null checker of headMap.firstEntry Verifying this change Make sure that the change passes the CI checks. This change is a trivial rework / code cleanup without any test coverage. Co-authored-by: gavingaozhangmin --- .../apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java index 5ea1346ca8962d..285b5d82513601 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerImpl.java @@ -48,6 +48,7 @@ import java.util.Queue; import java.util.Random; import java.util.Set; +import java.util.TreeMap; import java.util.UUID; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionException; @@ -3205,7 +3206,8 @@ public PositionImpl getPreviousPosition(PositionImpl position) { // The previous position will be the last position of an earlier ledgers NavigableMap headMap = ledgers.headMap(position.getLedgerId(), false); - if (headMap.isEmpty()) { + final Map.Entry firstEntry = headMap.firstEntry(); + if (firstEntry == null) { // There is no previous ledger, return an invalid position in the current ledger return PositionImpl.get(position.getLedgerId(), -1); } @@ -3213,13 +3215,13 @@ public PositionImpl getPreviousPosition(PositionImpl position) { // We need to find the most recent non-empty ledger for (long ledgerId : headMap.descendingKeySet()) { LedgerInfo li = headMap.get(ledgerId); - if (li.getEntries() > 0) { + if (li != null && li.getEntries() > 0) { return PositionImpl.get(li.getLedgerId(), li.getEntries() - 1); } } // in case there are only empty ledgers, we return a position in the first one - return PositionImpl.get(headMap.firstEntry().getKey(), -1); + return PositionImpl.get(firstEntry.getKey(), -1); } /**