This is an automated email from the ASF dual-hosted git repository.
aleksey pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/cassandra.git
The following commit(s) were added to refs/heads/trunk by this push:
new fb9e74a Fix buffer pool NPE with concurrent release due to
in-progress tiny pool eviction
fb9e74a is described below
commit fb9e74a4fe26eda988c0e98d578f7ded80a8c390
Author: Zhao Yang <[email protected]>
AuthorDate: Tue Apr 14 17:50:21 2020 +0800
Fix buffer pool NPE with concurrent release due to in-progress tiny pool
eviction
patch by Zhao Yang; reviewed by Aleksey Yeschenko for CASSANDRA-15726
---
CHANGES.txt | 1 +
.../apache/cassandra/utils/memory/BufferPool.java | 128 ++++++++++++---------
2 files changed, 73 insertions(+), 56 deletions(-)
diff --git a/CHANGES.txt b/CHANGES.txt
index f2d1a1f..d25ba64 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
4.0-alpha5
+ * Fix buffer pool NPE with concurrent release due to in-progress tiny pool
eviction (CASSANDRA-15726)
* Avoid race condition when completing stream sessions (CASSANDRA-15666)
* Flush with fast compressors by default (CASSANDRA-15379)
* Fix CqlInputFormat regression from the switch to system.size_estimates
(CASSANDRA-15637)
diff --git a/src/java/org/apache/cassandra/utils/memory/BufferPool.java
b/src/java/org/apache/cassandra/utils/memory/BufferPool.java
index 3fc7992..c0686c4 100644
--- a/src/java/org/apache/cassandra/utils/memory/BufferPool.java
+++ b/src/java/org/apache/cassandra/utils/memory/BufferPool.java
@@ -438,65 +438,81 @@ public class BufferPool
private <T> void removeIf(BiPredicate<Chunk, T> predicate, T value)
{
- switch (count)
+ // do not release matching chunks before we move null chunks to
the back of the queue;
+ // because, with current buffer release from another thread,
"chunk#release()" may eventually come back to
+ // "removeIf" causing NPE as null chunks are not at the back of
the queue.
+ Chunk toRelease0 = null, toRelease1 = null, toRelease2 = null;
+
+ try
{
- case 3:
- if (predicate.test(chunk2, value))
- {
- --count;
- Chunk chunk = chunk2;
- chunk2 = null;
- chunk.release();
- }
- case 2:
- if (predicate.test(chunk1, value))
- {
- --count;
- Chunk chunk = chunk1;
- chunk1 = null;
- chunk.release();
- }
- case 1:
- if (predicate.test(chunk0, value))
- {
- --count;
- Chunk chunk = chunk0;
- chunk0 = null;
- chunk.release();
- }
- break;
- case 0:
- return;
+ switch (count)
+ {
+ case 3:
+ if (predicate.test(chunk2, value))
+ {
+ --count;
+ toRelease2 = chunk2;
+ chunk2 = null;
+ }
+ case 2:
+ if (predicate.test(chunk1, value))
+ {
+ --count;
+ toRelease1 = chunk1;
+ chunk1 = null;
+ }
+ case 1:
+ if (predicate.test(chunk0, value))
+ {
+ --count;
+ toRelease0 = chunk0;
+ chunk0 = null;
+ }
+ break;
+ case 0:
+ return;
+ }
+ switch (count)
+ {
+ case 2:
+ // Find the only null item, and shift non-null so that
null is at chunk2
+ if (chunk0 == null)
+ {
+ chunk0 = chunk1;
+ chunk1 = chunk2;
+ chunk2 = null;
+ }
+ else if (chunk1 == null)
+ {
+ chunk1 = chunk2;
+ chunk2 = null;
+ }
+ break;
+ case 1:
+ // Find the only non-null item, and shift it to chunk0
+ if (chunk1 != null)
+ {
+ chunk0 = chunk1;
+ chunk1 = null;
+ }
+ else if (chunk2 != null)
+ {
+ chunk0 = chunk2;
+ chunk2 = null;
+ }
+ break;
+ }
}
- switch (count)
+ finally
{
- case 2:
- // Find the only null item, and shift non-null so that
null is at chunk2
- if (chunk0 == null)
- {
- chunk0 = chunk1;
- chunk1 = chunk2;
- chunk2 = null;
- }
- else if (chunk1 == null)
- {
- chunk1 = chunk2;
- chunk2 = null;
- }
- break;
- case 1:
- // Find the only non-null item, and shift it to chunk0
- if (chunk1 != null)
- {
- chunk0 = chunk1;
- chunk1 = null;
- }
- else if (chunk2 != null)
- {
- chunk0 = chunk2;
- chunk2 = null;
- }
- break;
+ if (toRelease0 != null)
+ toRelease0.release();
+
+ if (toRelease1 != null)
+ toRelease1.release();
+
+ if (toRelease2 != null)
+ toRelease2.release();
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
