{"push_id":1057996187,"size":1,"distinct_size":1,"ref":"refs/heads/lcts_dispose_cancel_race","head":"c9f917bc468902289c0c0414736c431a80c6d6c6","before":"a9beb6c5a5a00b16b77ce312da42b323df691b14","commits":[{"sha":"c9f917bc468902289c0c0414736c431a80c6d6c6","author":{"email":"94274d0b635748ad8df96c0fc4f19e1ecd3f67e2@microsoft.com","name":"stephentoub"},"message":"Fix race condition between CTS.Cancel and Dispose\n\nIn general, CancellationTokenSource.Dispose is not meant to be used concurrently with other operations on the instance: Dispose should only be used when the creator of the CTS is done with it.  However, there's one common situation where recommended usage runs afoul of this.  A typical usage of a LinkedCancellationTokenSource (as created by CancellationTokenSource.CreateLinkedTokenSource) is as follows:\n```C#\nprivate CancellationToken _internalCancellation;\n\npublic void SomeMethod(CancellationToken externalCancellation)\n{\n    using (var cts = CancellationTokenSource.CreateLinkedTokenSource(externalCancellation, _internalCancellation))\n    {\n        DoWork(cts.Token);\n    }\n}\n```\nwhere an LCTS is used to combine some external source of cancellation with some internal one.  It's important that the LCTS then be disposed, as otherwise it'll end up leaking state into constituent CTs, and in particular in the case of the externally provided one, that could be a very long lived token.  However, it's perfectly fine for the external token to have cancellation requested at any time during this method, which means that even though Dispose and Cancel weren't designed to be safe to execute concurrently, and even though we recommend against it, it's actually possible with recommended usage.\n\nIt's a very dificult race to reproduce, but when it does, the typical visible result is an ObjectDisposedException.  State corruption could also result.\n\nThis commit addresses that in two ways:\n1. Rather than having the LCTS register a delegate with each CT that calls Cancel, it instead has its delegate call NotifyCancellation.  This is the same method Cancel calls, but without the upfront disposal check.  That eliminates the easily visible result of a race that would cause an ObjectDisposedException.\n2. With all of the tweaking that's been done to Dispose since it was originally written, it's actually very close to being safe to use concurrently with Cancel; this takes it the rest of the way (though we still don't want to document it as such, it's really just to make LCTS work correctly).  The only state Dispose mutates that could be problematic is a Timer and a ManualResetEvent.  Timer's Dispose is already thread-safe.  MRE's isn't, so if it's been allocated (which is relatively rare), I add a single Interlocked.Exchange to null it out, and then do a volatile read on a state field to see whether cancellation is currently in progress, only Dispose'ing of the MRE if it is, and leaving it for finalization otherwise.  On the Cancel side, we just add some null checks to ensure we're not attempting to Dispose of null'd out fields, again with volatile reads where necessary.","distinct":true,"url":"https://api.github.com/repos/stephentoub/coreclr/commits/c9f917bc468902289c0c0414736c431a80c6d6c6"}]}