Skip to content

Fix scan iterator [dev]#1718

Merged
badrishc merged 1 commit intodevfrom
badrishc/fix-scan-iterator
Apr 21, 2026
Merged

Fix scan iterator [dev]#1718
badrishc merged 1 commit intodevfrom
badrishc/fix-scan-iterator

Conversation

@badrishc
Copy link
Copy Markdown
Collaborator

Fix deferred DoReadPage drain callback accessing disposed ScanIteratorBase

BufferAndLoad registers DoReadPage as a deferred drain callback via BumpCurrentEpoch. This callback captures instance state including loadCompletionEvents. When the callback is for a read-ahead page and SafeToReclaimEpoch hasn't advanced yet, the callback remains in the drain list. If the scan completes and the iterator is disposed before the callback executes, loadCompletionEvents is set to null. When the callback later runs (during another thread's Drain), it accesses the null array, throwing NullReferenceException.

This exception from a drain callback leaves the epoch in a held state (SuspendDrain's Resume/Release is not exception-safe), causing the cascading 'Trying to acquire protected epoch' assertion failure seen in ObjectIterationPushLockTest(4,4,Iterate,False).

Fix: capture loadCompletionEvents into a local before accessing it in DoReadPage, and return early if the iterator has been disposed.

…rBase

BufferAndLoad registers DoReadPage as a deferred drain callback via
BumpCurrentEpoch. This callback captures instance state including
loadCompletionEvents. When the callback is for a read-ahead page and
SafeToReclaimEpoch hasn't advanced yet, the callback remains in the
drain list. If the scan completes and the iterator is disposed before
the callback executes, loadCompletionEvents is set to null. When the
callback later runs (during another thread's Drain), it accesses the
null array, throwing NullReferenceException.

This exception from a drain callback leaves the epoch in a held state
(SuspendDrain's Resume/Release is not exception-safe), causing the
cascading 'Trying to acquire protected epoch' assertion failure seen
in ObjectIterationPushLockTest(4,4,Iterate,False).

Fix: capture loadCompletionEvents into a local before accessing it in
DoReadPage, and return early if the iterator has been disposed.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 21, 2026 05:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a concurrency issue in Tsavorite scan iterators where a deferred epoch drain callback could run after the iterator is disposed, leading to a NullReferenceException and potentially leaving the epoch in a held state.

Changes:

  • In ScanIteratorBase.BufferAndLoad, updates the deferred DoReadPage callback to snapshot loadCompletionEvents into a local variable and return early if it has been nulled by Dispose().
  • Adds explanatory comments documenting why the deferred callback can outlive the iterator instance state.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@badrishc badrishc merged commit b86f828 into dev Apr 21, 2026
34 checks passed
@badrishc badrishc deleted the badrishc/fix-scan-iterator branch April 21, 2026 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants