dragonflydb / dragonflydb/dragonfly

P1 — `NotifyPending()` is reentrant through a suspending expiry checker

Open
#8,068 1 comment 0 reactions 0 assignees View on GitHub
bug
Dominant language
C++
Stars
31.5k
Forks
1.3k
Avg merge
1d 10h
Merged PRs (30d)
137

Description

`NotifyPending()` iterates the live `awakened_indices_` / `awakened_keys` sets with no
reentrancy guard and no snapshot (`blocking_controller.cc:150-182`). The stream readiness
checker calls `FindReadOnly()` (`stream_family.cc:3032`), which can enter
`ExpireIfNeeded()` (`db_slice.cc:1451-1493`); that path can suspend **before** deleting
the entry — in `RecordExpiryBlocking()` when a replication journal is active
(`db_slice.cc:1471-1473`), or in keyspace-notification delivery under publish-buffer
backpressure (`db_slice.cc:1476-1484`, `channel_store.cc:120`). While the checker fiber is
suspended, any concluding transaction on the shard calls `NotifyPending()` again
(`transaction.cc:668`). The inner call clears and erases the containers the outer frames
still reference — including the `DbWatchTable` and `WatchQueue` held by reference — so the
outer call resumes on invalidated iterators and a stale `PrimeIterator`.

Consequences: lost wake, CHECK/DCHECK failure, use-after-free, duplicate erase.

Reachability: requires an active replication journal with a stalled streamer, or
`notify-keyspace-events Ex` with a backpressured subscriber. A default standalone
configuration has no suspension point on this path. The synchronous same-fiber lazy-expiry
case is benign and already covered by `XReadGroupBlockLazyExpireDuringWakeDoesNotCrash`;
the uncovered defect is the cross-fiber suspension window.

Scenario (race-sensitive):

```text
CONFIG SET notify-keyspace-events Ex
PSUBSCRIBE __keyevent@0__:expired # subscriber stops consuming replies
# fill the subscriber's output budget so an expired event must wait

XGROUP CREATE s g 0 MKSTREAM
XREADGROUP GROUP g c BLOCK 0 STREAMS s > # client A
PEXPIRE s 10 # client B
# after the TTL elapses, run another write on the same shard
SET unrelated-key value
```

A deterministic test needs a latch inside `RecordExpiryBlocking()` or the notification
send; a timing-only test would be flaky.

Contributor guide

Open the contributing guide

Research direction

Read blocking_controller.cc:150-182 and trace the calls through stream_family.cc:3032, db_slice.cc:1451-1493, RecordExpiryBlocking(), channel_store.cc:120, and transaction.cc:668. Use the existing XReadGroupBlockLazyExpireDuringWakeDoesNotCrash test as a starting point, then add a deterministic latch at the suspension point and verify that reentrant NotifyPending() does not invalidate the outer iteration or lose wakeups.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.