From c251136278fcbcc63aae52c73fbd5328b2f878f5 Mon Sep 17 00:00:00 2001 From: Ludovic Henry Date: Tue, 29 Sep 2015 12:46:31 +0100 Subject: [PATCH] Fix dispose race condition in CancellationTokenSource The race condition would manifest in the following code: ``` for (int i = 0, total = 100000; i < total; ++i) { if (i % 50 == 0) Console.WriteLine ("{0}/{1}", i, total); var c1 = new CancellationTokenSource (); var wh = c1.Token.WaitHandle; c1.CancelAfter (1); Thread.Sleep (1); c1.Dispose (); } ``` And we would observe the following exception: ``` Unhandled Exception: System.ObjectDisposedException: Cannot access a disposed object. Object name: 'System.Threading.ManualResetEvent'. at System.Threading.WaitHandle.CheckDisposed () [0x00016] in /Users/builder/data/lanes/1196/33e00ac6/source/mono/mcs/class/corlib/System.Threading/WaitHandle.cs:426 at System.Threading.EventWaitHandle.Set () [0x0000c] in /Users/builder/data/lanes/1196/33e00ac6/source/mono/mcs/class/corlib/System.Threading/EventWaitHandle.cs:205 at (wrapper remoting-invoke-with-check) System.Threading.EventWaitHandle:Set () at System.Threading.CancellationTokenSource.NotifyCancellation (Boolean throwOnFirstException) [0x00051] in /Users/builder/data/lanes/1196/33e00ac6/source/mono/external/referencesource/mscorlib/system/threading/CancellationTokenSource.cs:723 at System.Threading.CancellationTokenSource.Cancel (Boolean throwOnFirstException) [0x00006] in /Users/builder/data/lanes/1196/33e00ac6/source/mono/external/referencesource/mscorlib/system/threading/CancellationTokenSource.cs:409 at System.Threading.CancellationTokenSource.Cancel () <0x7d75ada8 + 0x00017> in :0 at System.Threading.CancellationTokenSource.TimerCallbackLogic (System.Object obj) [0x00012] in /Users/builder/data/lanes/1196/33e00ac6/source/mono/external/referencesource/mscorlib/system/threading/CancellationTokenSource.cs:538 ``` This would be a race condition between the TimerCallbackLogic call to Cancel and the call to Dispose on another thread. You could trigger it more reliably by putting a Thread.Sleep at CancellationTokenSource.cs:599. --- .../system/threading/CancellationTokenSource.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/mcs/class/referencesource/mscorlib/system/threading/CancellationTokenSource.cs b/mcs/class/referencesource/mscorlib/system/threading/CancellationTokenSource.cs index f5e97cba453..a0a682b8598 100644 --- a/mcs/class/referencesource/mscorlib/system/threading/CancellationTokenSource.cs +++ b/mcs/class/referencesource/mscorlib/system/threading/CancellationTokenSource.cs @@ -71,7 +71,7 @@ namespace System.Threading /// actually run the callbacks. private volatile int m_threadIDExecutingCallbacks = -1; - private bool m_disposed; + private int m_disposed; private CancellationTokenRegistration [] m_linkingRegistrations; //lazily initialized if required. @@ -134,7 +134,7 @@ namespace System.Threading /// internal bool IsDisposed { - get { return m_disposed; } + get { return m_disposed == 1; } } /// @@ -573,7 +573,7 @@ namespace System.Threading // mutates a sparseArrayFragment and then reads from properties of the CTS that are not // invalidated by cts.Dispose(). - if (m_disposed) + if (m_disposed != 0 || Interlocked.CompareExchange (ref m_disposed, 1, 0) != 0) return; if (m_timer != null) m_timer.Dispose(); @@ -598,8 +598,6 @@ namespace System.Threading m_kernelEvent.Close(); // the critical cleanup to release an OS handle m_kernelEvent = null; // free for GC. } - - m_disposed = true; } } @@ -613,7 +611,7 @@ namespace System.Threading #endif internal void ThrowIfDisposed() { - if (m_disposed) + if (m_disposed == 1) ThrowObjectDisposedException(); } -- 2.25.1