Skip to content

Commit 42e2d0d

Browse files
badrishcCopilot
andcommitted
Guard deferred DoReadPage callback against disposed scan iterator
BufferAndLoad defers page reads via BumpCurrentEpoch(() => DoReadPage(...)). These drain callbacks capture 'this' and access instance state (frame memory, loadCompletionEvents, etc.). For read-ahead pages, the callback may still be in the epoch drain list when the scan completes and the iterator is disposed. The callback then accesses freed native frame memory (AccessViolationException) or null arrays (NullReferenceException), and the resulting exception from within a drain callback leaves the epoch in a held state, causing cascading 'Trying to acquire protected epoch' assertion failures. Fix: add a volatile isDisposed flag that is set at the start of Dispose(), before any resources are freed. DoReadPage checks this flag and returns early if the iterator has been disposed. This is a simple, race-free guard that requires no epoch manipulation in Dispose. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent b86f828 commit 42e2d0d

1 file changed

Lines changed: 13 additions & 6 deletions

File tree

libs/storage/Tsavorite/cs/src/core/Allocator/ScanIteratorBase.cs

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ public abstract class ScanIteratorBase
2828
/// <summary>Epoch from the store</summary>
2929
protected readonly LightEpoch epoch;
3030

31+
/// <summary>Set when the iterator is disposed; deferred drain callbacks check this to avoid accessing freed resources.</summary>
32+
volatile bool isDisposed;
33+
3134
/// <summary>Current address for iteration</summary>
3235
protected long currentAddress;
3336
/// <summary>Next address for iteration</summary>
@@ -208,14 +211,13 @@ protected bool BufferAndLoad(long currentIterationAddress, long currentPage, lon
208211

209212
void DoReadPage(int frameIndex)
210213
{
211-
// The drain callback may execute after the iterator has been disposed (loadCompletionEvents set to null),
212-
// because the callback is deferred via BumpCurrentEpoch and only runs when SafeToReclaimEpoch advances.
213-
// This can happen for read-ahead pages (frameIndex > 0) when the scan completes before the callback runs.
214-
var events = loadCompletionEvents;
215-
if (events is null)
214+
// This callback may be deferred via BumpCurrentEpoch and execute after the
215+
// iterator is disposed. Check the disposed flag to avoid accessing freed
216+
// frame memory (AV) or disposed arrays.
217+
if (isDisposed)
216218
return;
217219
AsyncReadPageFromDeviceToFrame(readBuffer, readPage: frameIndex + GetPageOfAddress(currentIterationAddress, logPageSizeBits), untilAddress: endIterationAddress,
218-
context: Empty.Default, out events[nextFrame], devicePageOffset: 0, device: null, objectLogDevice: null, loadCTSs[nextFrame]);
220+
context: Empty.Default, out loadCompletionEvents[nextFrame], devicePageOffset: 0, device: null, objectLogDevice: null, loadCTSs[nextFrame]);
219221
loadedPages[nextFrame] = pageEndAddress;
220222
}
221223
}
@@ -322,6 +324,11 @@ private bool WaitForFrameLoad(long currentAddress, long currentFrame)
322324
/// </summary>
323325
public virtual void Dispose()
324326
{
327+
// Mark as disposed before freeing resources. Deferred DoReadPage callbacks (registered via
328+
// BumpCurrentEpoch in BufferAndLoad) check this flag and no-op if set, avoiding access to
329+
// freed frame memory (AV) or disposed arrays (NRE).
330+
isDisposed = true;
331+
325332
for (var i = 0; i < frameSize; i++)
326333
{
327334
// Wait for ongoing reads to complete/fail; if the wait throws (e.g. due to cancellation), we still

0 commit comments

Comments
 (0)