Skip to content

Commit 12a30b5

Browse files
committed
CNDB-14182: Add some null checks in InMemoryTrie
Currently returning a null from an Upserter may corrupt the trie state which may end up in a serious problem. It is better to crash instead.
1 parent 708b56d commit 12a30b5

File tree

2 files changed

+15
-8
lines changed

2 files changed

+15
-8
lines changed

src/java/org/apache/cassandra/db/tries/InMemoryTrie.java

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,10 @@
2222
import java.util.concurrent.atomic.AtomicReferenceArray;
2323
import java.util.function.Predicate;
2424

25+
import javax.annotation.Nonnull;
26+
2527
import com.google.common.annotations.VisibleForTesting;
28+
import com.google.common.base.Preconditions;
2629
import com.google.common.base.Predicates;
2730

2831
import org.agrona.concurrent.UnsafeBuffer;
@@ -296,8 +299,9 @@ private int allocateNewObject()
296299
* @return A content id that can be used to reference the content, encoded as ~index where index is the
297300
* position of the value in the content array.
298301
*/
299-
private int addContent(T value) throws TrieSpaceExhaustedException
302+
private int addContent(@Nonnull T value) throws TrieSpaceExhaustedException
300303
{
304+
Preconditions.checkNotNull(value, "Content value cannot be null");
301305
int index = objectAllocator.allocate();
302306
int leadBit = getBufferIdx(index, CONTENTS_START_SHIFT, CONTENTS_START_SIZE);
303307
int ofs = inBufferOffset(index, leadBit, CONTENTS_START_SIZE);
@@ -1329,9 +1333,9 @@ public interface UpsertTransformerWithKeyProducer<T, U>
13291333
* @param existing Existing content for this key, or null if there isn't any.
13301334
* @param update The update, always non-null.
13311335
* @param keyState An interface that can be used to retrieve the path of the value being updated.
1332-
* @return The combined value to use.
1336+
* @return The combined value to use. Cannot be null.
13331337
*/
1334-
T apply(T existing, U update, KeyProducer<T> keyState);
1338+
@Nonnull T apply(T existing, @Nonnull U update, @Nonnull KeyProducer<T> keyState);
13351339
}
13361340

13371341
/**
@@ -1353,7 +1357,7 @@ public interface UpsertTransformer<T, U> extends UpsertTransformerWithKeyProduce
13531357
* @param update The update, always non-null.
13541358
* @return The combined value to use. Cannot be null.
13551359
*/
1356-
T apply(T existing, U update);
1360+
@Nonnull T apply(T existing, @Nonnull U update);
13571361

13581362
/**
13591363
* Version of the above that also provides the path of a value being updated.
@@ -1363,7 +1367,7 @@ public interface UpsertTransformer<T, U> extends UpsertTransformerWithKeyProduce
13631367
* @param keyState An interface that can be used to retrieve the path of the value being updated.
13641368
* @return The combined value to use. Cannot be null.
13651369
*/
1366-
default T apply(T existing, U update, KeyProducer<T> keyState)
1370+
default @Nonnull T apply(T existing, @Nonnull U update, @Nonnull KeyProducer<T> keyState)
13671371
{
13681372
return apply(existing, update);
13691373
}
@@ -1441,7 +1445,10 @@ void applyContent() throws TrieSpaceExhaustedException
14411445
{
14421446
T existingContent = state.getContent();
14431447
T combinedContent = transformer.apply(existingContent, content, state);
1444-
state.setContent(combinedContent, // can be null
1448+
if (combinedContent == null)
1449+
throw new AssertionError("Transformer " + transformer + " returned null content for "
1450+
+ existingContent + ", " + content);
1451+
state.setContent(combinedContent,
14451452
state.currentDepth >= forcedCopyDepth); // this is called at the start of processing
14461453
}
14471454
}

test/unit/org/apache/cassandra/db/tries/CellReuseTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -304,9 +304,9 @@ static void addThrowingEntry(ByteComparable b,
304304
{
305305
if (upd instanceof Boolean)
306306
{
307-
if (upd != null && !((Boolean) upd))
307+
if (!((Boolean) upd))
308308
throw new TestException();
309-
return null;
309+
return true;
310310
}
311311
else
312312
return upd;

0 commit comments

Comments
 (0)