Skip to content

Conversation

@AArnott
Copy link
Member

@AArnott AArnott commented Jan 25, 2019

I made a few changes for this:

  1. Added another couple of checks to reduce the likelihood of the race condition and reduce work where it can be optimized away.
  2. Within the task.ContinueWith delegate, I check whether we are executing inline. If we are, we divert the CTR.Dispose call to the threadpool.
  3. We only reschedule the work to the threadpool if the CancellationToken hasn't already been canceled, since once it has, there is no need to dispose of the CTR anyway since all the memory has been released. So I added a check to skip the threadpool scheduling where we can.

That last point means that basically the ThreadPool.QueueUserWorkItem delegate is dead code, since the only caller for AttachCancellation right now will never complete the TaskCompletionSource while this method is executing, leaving only the cancellation case. But since this method might be used in other places in the future, I thought we should make sure it is fully implemented. I used the debugger to manually test that code path to ensure correctness.

Fixes #458

@AArnott AArnott added this to the v15.8 milestone Jan 25, 2019
@AArnott AArnott self-assigned this Jan 25, 2019
@AArnott AArnott closed this Jan 25, 2019
@AArnott AArnott reopened this Jan 25, 2019
@AArnott AArnott merged commit a9e9c4f into microsoft:v15.8 Jan 25, 2019
@AArnott AArnott deleted the fix458 branch January 25, 2019 19:06
AArnott added a commit that referenced this pull request Dec 11, 2025
* Update mcr.microsoft.com/dotnet/sdk Docker tag to v10.0.101

* Bump .NET SDK

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Andrew Arnott <andrew.arnott@microsoft.com>
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