Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 14, 2026

The CancelCTS() method in SampleContainer had a race condition where concurrent calls could cause double-disposal of CancellationTokenSource. The pattern of reading, nulling, and disposing without synchronization allowed multiple threads to read the same non-null reference before any thread nulled it.

Changes:

  • Added _ctsLock object to serialize all _sampleLoadingCts access
  • Wrapped read-and-null operations in CancelCTS() with lock
  • Protected CancellationTokenSource creation/disposal in LoadSampleAsync with lock

Before:

private void CancelCTS()
{
    var cts = _sampleLoadingCts;
    if (cts == null) return;
    
    _sampleLoadingCts = null;  // Race: multiple threads can read before this executes
    using (cts) { cts.Cancel(); }
}

After:

private void CancelCTS()
{
    CancellationTokenSource? cts;
    lock (_ctsLock)
    {
        cts = _sampleLoadingCts;
        _sampleLoadingCts = null;  // Atomic with read
    }
    
    if (cts != null)
    {
        using (cts) { cts.Cancel(); }
    }
}

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Co-authored-by: weiyuanyue <176483933+weiyuanyue@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix feedback on IDisposable analyzer warnings Fix race condition in CancellationTokenSource disposal Jan 14, 2026
Copilot AI requested a review from weiyuanyue January 14, 2026 08:29
@weiyuanyue weiyuanyue marked this pull request as ready for review January 15, 2026 10:12
@weiyuanyue weiyuanyue merged commit d8aa456 into milly/idisp Jan 15, 2026
1 check passed
@weiyuanyue weiyuanyue deleted the copilot/sub-pr-545-again branch January 15, 2026 10:12
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