From 9e9fbe0d1cf37bb077509203aafef2bb3b9ef4c1 Mon Sep 17 00:00:00 2001 From: clooker Date: Fri, 27 Aug 2021 15:50:18 +0800 Subject: [PATCH 1/3] [ISSUE 11796] throw NPE when readEntry --- .../bookkeeper/mledger/impl/ManagedCursorImpl.java | 9 +++++++-- .../bookkeeper/mledger/impl/ManagedLedgerImpl.java | 1 + .../org/apache/bookkeeper/mledger/impl/OpReadEntry.java | 2 +- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java index 82b4cfcc9b68b..1db3049c9aea6 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java @@ -622,9 +622,11 @@ public void asyncReadEntries(int numberOfEntriesToRead, long maxSizeBytes, ReadE int numOfEntriesToRead = applyMaxSizeCap(numberOfEntriesToRead, maxSizeBytes); - PENDING_READ_OPS_UPDATER.incrementAndGet(this); OpReadEntry op = OpReadEntry.create(this, readPosition, numOfEntriesToRead, callback, ctx, maxPosition); - ledger.asyncReadEntries(op); + if (op.readPosition != null) { + PENDING_READ_OPS_UPDATER.incrementAndGet(this); + ledger.asyncReadEntries(op); + } } @Override @@ -762,6 +764,9 @@ public void asyncReadEntriesOrWait(int maxEntries, long maxSizeBytes, ReadEntrie } else { OpReadEntry op = OpReadEntry.create(this, readPosition, numberOfEntriesToRead, callback, ctx, maxPosition); + if (op.readPosition == null) { + return; + } if (!WAITING_READ_OP_UPDATER.compareAndSet(this, null, op)) { callback.readEntriesFailed(new ManagedLedgerException("We can only have a single waiting callback"), 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 0823886e0bdbb..db494a03f1329 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 @@ -2141,6 +2141,7 @@ PositionImpl startReadOperationOnLedger(PositionImpl position, OpReadEntry opRea if (null == ledgerId) { opReadEntry.readEntriesFailed(new ManagedLedgerException.NoMoreEntriesToReadException("The ceilingKey(K key) method is used to return the " + "least key greater than or equal to the given key, or null if there is no such key"), null); + return null; } if (ledgerId != position.getLedgerId()) { diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java index 91a6e26f567d0..f794ecde81497 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java @@ -48,7 +48,6 @@ class OpReadEntry implements ReadEntriesCallback { public static OpReadEntry create(ManagedCursorImpl cursor, PositionImpl readPositionRef, int count, ReadEntriesCallback callback, Object ctx, PositionImpl maxPosition) { OpReadEntry op = RECYCLER.get(); - op.readPosition = cursor.ledger.startReadOperationOnLedger(readPositionRef, op); op.cursor = cursor; op.count = count; op.callback = callback; @@ -59,6 +58,7 @@ public static OpReadEntry create(ManagedCursorImpl cursor, PositionImpl readPosi op.maxPosition = maxPosition; op.ctx = ctx; op.nextReadPosition = PositionImpl.get(op.readPosition); + op.readPosition = cursor.ledger.startReadOperationOnLedger(readPositionRef, op); return op; } From 1815f072ef142ac8e9603b227c576fd1e1079d2f Mon Sep 17 00:00:00 2001 From: clooker Date: Mon, 6 Sep 2021 15:22:08 +0800 Subject: [PATCH 2/3] [ISSUE 11796] check if op init success --- .../java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java index f794ecde81497..a2b2d597a5b8e 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java @@ -57,8 +57,10 @@ public static OpReadEntry create(ManagedCursorImpl cursor, PositionImpl readPosi } op.maxPosition = maxPosition; op.ctx = ctx; - op.nextReadPosition = PositionImpl.get(op.readPosition); op.readPosition = cursor.ledger.startReadOperationOnLedger(readPositionRef, op); + if (op.readPosition != null) { + op.nextReadPosition = PositionImpl.get(op.readPosition); + } return op; } From 5b2e43546a9f9cdb3904797e604039eac65ce7c4 Mon Sep 17 00:00:00 2001 From: clooker Date: Fri, 10 Sep 2021 12:09:28 +0800 Subject: [PATCH 3/3] [ISSUE 11796] op may use after recycled --- .../apache/bookkeeper/mledger/impl/ManagedCursorImpl.java | 4 ++-- .../org/apache/bookkeeper/mledger/impl/OpReadEntry.java | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java index 1db3049c9aea6..02a6b03b943a9 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedCursorImpl.java @@ -623,7 +623,7 @@ public void asyncReadEntries(int numberOfEntriesToRead, long maxSizeBytes, ReadE int numOfEntriesToRead = applyMaxSizeCap(numberOfEntriesToRead, maxSizeBytes); OpReadEntry op = OpReadEntry.create(this, readPosition, numOfEntriesToRead, callback, ctx, maxPosition); - if (op.readPosition != null) { + if (op != null) { PENDING_READ_OPS_UPDATER.incrementAndGet(this); ledger.asyncReadEntries(op); } @@ -764,7 +764,7 @@ public void asyncReadEntriesOrWait(int maxEntries, long maxSizeBytes, ReadEntrie } else { OpReadEntry op = OpReadEntry.create(this, readPosition, numberOfEntriesToRead, callback, ctx, maxPosition); - if (op.readPosition == null) { + if (op == null) { return; } diff --git a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java index a2b2d597a5b8e..78cf0d01390f9 100644 --- a/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java +++ b/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpReadEntry.java @@ -57,10 +57,12 @@ public static OpReadEntry create(ManagedCursorImpl cursor, PositionImpl readPosi } op.maxPosition = maxPosition; op.ctx = ctx; - op.readPosition = cursor.ledger.startReadOperationOnLedger(readPositionRef, op); - if (op.readPosition != null) { - op.nextReadPosition = PositionImpl.get(op.readPosition); + PositionImpl position = cursor.ledger.startReadOperationOnLedger(readPositionRef, op); + if (position == null) { + return null; } + op.readPosition = position; + op.nextReadPosition = PositionImpl.get(op.readPosition); return op; }