[fix][broker] Prevent NPE when the last ACK races with sticky hash reassignment - #26471
Open
void-ptr974 wants to merge 2 commits into
Open
[fix][broker] Prevent NPE when the last ACK races with sticky hash reassignment#26471void-ptr974 wants to merge 2 commits into
void-ptr974 wants to merge 2 commits into
Conversation
…assignment Assisted-by: Codex
Add deterministic coverage for entry publication, concurrent reference additions, and final ACK slow-path rechecks. Exercise nested batching and closing during concurrent removals using an injectable test lock. Validation: 57 tracker and pending-ack test invocations, 4 broker regression test invocations, and offline quickCheck passed. Targeted implementation mutations fail the three new tests as expected. Assisted-by: Codex
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
DrainingHashesTracker.reduceRefCountpreviously looked up an entry, decremented its reference count,and removed it from the map as separate operations. If the sticky hash was reassigned to its original
consumer in between,
shouldBlockStickyKeyHashcould remove the entry first. The final ACK then receivednullfromdrainingHashes.removeand dereferenced it while checkingisBlocking, causing an NPE.The same key-only removal could also let a stale ACK remove a replacement entry or repeat stats cleanup.
Modifications
write lock.
ACKs, and concurrent reference reductions.
Verifying this change
DrainingHashesTrackerConcurrencyTest.DrainingHashesTrackerTest,DrainingHashesTrackerConcurrencyTest, andPendingAcksMapTest(48 test invocations, no failures).
./gradlew --offline quickCheck.An exploratory tracker-level JMH comparison against the exact base revision found no material regression
from the CAS version. The one-thread tracked-ACK result changed by -0.9%; the four-thread mixed result by
-2.7%; sampled mixed p99 remained approximately 3.3-3.6 microseconds. These figures exclude the rest of
broker ACK processing.
Does this pull request potentially affect one of the following parts:
The change narrows write-lock use to entry lifecycle transitions and uses CAS for non-final reference
decrements.