Skip to content

Commit c520bb0

Browse files
dougqhclaude
andcommitted
Guard MutatingBucketIterator.replace against relinking an already-linked Entry
bric3 hit the case while trying the Hashtable API: splicing in an Entry that's still part of another chain silently corrupts it. insertHeadEntryAt already asserted against this; add the same assertion to replace(), note the caller contract on D1.insert's javadoc, and add a regression test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent d05c61f commit c520bb0

2 files changed

Lines changed: 28 additions & 1 deletion

File tree

‎internal-api/src/main/java/datadog/trace/util/Hashtable.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,10 @@ public TEntry remove(@Nullable K key) {
253253
/**
254254
* Unconditionally adds {@code newEntry} ({@code true}), or {@code false} if the table is
255255
* already at capacity. Caller-responsible: {@code newEntry}'s key must be absent, else it lands
256-
* shadowed behind the existing entry.
256+
* shadowed behind the existing entry. {@code newEntry} must also be a fresh entry, not already
257+
* linked into this or any other table's bucket chain -- reinserting an already-linked entry
258+
* corrupts the chain it's still part of (guarded by an assertion in {@link
259+
* #insertHeadEntryAt(Hashtable.Entry[], int, Hashtable.Entry)}).
257260
*/
258261
public boolean insert(@Nonnull TEntry newEntry) {
259262
return insertHeadEntryFor(this.sizeManager, this.buckets, newEntry.keyHash, newEntry);
@@ -1802,6 +1805,8 @@ public void replace(@Nonnull TEntry replacementEntry) {
18021805
if (oldCurEntry == null) {
18031806
throw new IllegalStateException();
18041807
}
1808+
assert replacementEntry.next() == null
1809+
: "Entry already linked -- inserting the same Entry instance twice corrupts the chain";
18051810

18061811
Hashtable.Entry oldNext = oldCurEntry.next();
18071812
replacementEntry.setNext(oldNext);

‎internal-api/src/test/java/datadog/trace/util/HashtableTest.java‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static org.junit.jupiter.api.Assertions.assertSame;
1111
import static org.junit.jupiter.api.Assertions.assertThrows;
1212
import static org.junit.jupiter.api.Assertions.assertTrue;
13+
import static org.junit.jupiter.api.Assumptions.assumeTrue;
1314

1415
import datadog.trace.util.Hashtable.BucketIterator;
1516
import datadog.trace.util.Hashtable.MutatingBucketIterator;
@@ -200,6 +201,21 @@ void insertHeadEntrySplicesAsNewHead() {
200201
assertSame(a, b.next());
201202
assertNull(a.next());
202203
}
204+
205+
@Test
206+
void insertHeadEntryOfAlreadyLinkedEntryTripsAssertion() {
207+
assumeTrue(assertionsEnabled(), "assert-guard test requires -ea");
208+
Hashtable.State<StringIntEntry> table = Hashtable.createCapped(4);
209+
Hashtable.Entry[] buckets = table.buckets;
210+
StringIntEntry a = new StringIntEntry("a", 1);
211+
StringIntEntry b = new StringIntEntry("b", 2);
212+
Hashtable.insertHeadEntryAt(buckets, 0, a);
213+
Hashtable.insertHeadEntryAt(buckets, 0, b); // chain is now b -> a, so b.next() != null
214+
assertThrows(
215+
AssertionError.class,
216+
() -> Hashtable.insertHeadEntryAt(buckets, 1, b),
217+
"re-inserting an already-linked entry corrupts the chain and must be caught");
218+
}
203219
}
204220

205221
// ============ Deprecated Support facade ============
@@ -923,4 +939,10 @@ void tableSizeManagerOperatesOnItsOwnBuckets() {
923939
assertNull(table.buckets[0]);
924940
}
925941
}
942+
943+
private static boolean assertionsEnabled() {
944+
boolean enabled = false;
945+
assert enabled = true;
946+
return enabled;
947+
}
926948
}

0 commit comments

Comments
 (0)