tenantcapabilitieswatcher: fix nil deref removing placeholder entry#168391
Merged
trunk-io[bot] merged 1 commit intocockroachdb:masterfrom Apr 30, 2026
Merged
Conversation
The Watcher's getInternal method inserts a placeholder watcherEntry with a nil embedded *Entry pointer when a reader queries capabilities for a tenant not yet seen by the rangefeed. If removeEntryForTenantIDLocked was later called for that tenant, it accessed entry.Name through the nil embedded pointer, causing a panic. This can occur when a rangefeed restart re-delivers a delete event for a tenant whose entry was already removed during a previous rangefeed lifetime. If a reader called GetInfo for that tenant in the interim, the store contains only the nil-Entry placeholder, and the re-delivered delete dereferences it. Epic: none Release note (bug fix): Fixed a rare nil pointer dereference panic in the tenant capabilities watcher that could occur when a tenant entry was removed before it was fully populated by the rangefeed. Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
Contributor
|
😎 Merged successfully - details. |
Member
|
This change is |
stevendanna
added
the
backport-all
stevendanna
commented
Apr 16, 2026
| // duplicate delete. A duplicate delete interleaved with a | ||
| // getInternal call can also result is us finding an entry | ||
| // with a nil value for the embedded Entry struct. | ||
| if entry, ok := w.mu.store[tid]; ok && entry.Entry != nil { |
Collaborator
Author
There was a problem hiding this comment.
There is another question about the downstream affects of notifying a change of a tenant (from a previous GetInfo call) that no longer exists. But, we still shouldn't panic here in that case.
stevendanna
requested review from
cthumuluru-crdb and
shubhamdhama
and removed request for
cthumuluru-crdb
arulajmani
approved these changes
Apr 20, 2026
Collaborator
arulajmani
left a comment
arulajmani
left a comment
There was a problem hiding this comment.
@arulajmani reviewed 3 files and all commit messages, and made 1 comment.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on shubhamdhama).
Collaborator
Author
|
/trunk merge |
This was referenced Apr 30, 2026
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
3 participants
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.
The Watcher's getInternal method inserts a placeholder watcherEntry
with a nil embedded *Entry pointer when a reader queries capabilities
for a tenant not yet seen by the rangefeed. If removeEntryForTenantIDLocked
was later called for that tenant, it accessed entry.Name through the nil
embedded pointer, causing a panic.
This can occur when a rangefeed restart re-delivers a delete event for
a tenant whose entry was already removed during a previous rangefeed
lifetime. If a reader called GetInfo for that tenant in the interim,
the store contains only the nil-Entry placeholder, and the re-delivered
delete dereferences it.
Epic: none