From 70f1dfda16cd97fc2a6d6c953396250771e521c2 Mon Sep 17 00:00:00 2001 From: David Karnok Date: Sat, 26 May 2018 17:32:42 +0200 Subject: [PATCH] 4.x: SerialDisposable to use lock free methods (#535) * Lockfree SerialDisposable * Disposed SerialDisposable returns null from Disposable property --- .../Disposables/SerialDisposable.cs | 61 ++++++++----------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/Rx.NET/Source/src/System.Reactive/Disposables/SerialDisposable.cs b/Rx.NET/Source/src/System.Reactive/Disposables/SerialDisposable.cs index 30ae7cf6eb..4f69c14859 100644 --- a/Rx.NET/Source/src/System.Reactive/Disposables/SerialDisposable.cs +++ b/Rx.NET/Source/src/System.Reactive/Disposables/SerialDisposable.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the Apache 2.0 License. // See the LICENSE file in the project root for more information. +using System.Threading; + namespace System.Reactive.Disposables { /// @@ -9,12 +11,10 @@ namespace System.Reactive.Disposables /// public sealed class SerialDisposable : ICancelable { - private readonly object _gate = new object(); private IDisposable _current; - private bool _disposed; /// - /// Initializes a new instance of the class. + /// Initializes a new instance of the class. /// public SerialDisposable() { @@ -27,10 +27,10 @@ public bool IsDisposed { get { - lock (_gate) - { - return _disposed; - } + // We use a sentinel value to indicate we've been disposed. This sentinel never leaks + // to the outside world (see the Disposable property getter), so no-one can ever assign + // this value to us manually. + return Volatile.Read(ref _current) == BooleanDisposable.True; } } @@ -42,28 +42,32 @@ public IDisposable Disposable { get { - return _current; + var a = Volatile.Read(ref _current); + // Don't leak the DISPOSED sentinel + if (a == BooleanDisposable.True) + { + a = null; + } + return a; } set { - var shouldDispose = false; - var old = default(IDisposable); - lock (_gate) + var copy = Volatile.Read(ref _current); + for (;;) { - shouldDispose = _disposed; - if (!shouldDispose) + if (copy == BooleanDisposable.True) { - old = _current; - _current = value; + value?.Dispose(); + return; } - } - - old?.Dispose(); - - if (shouldDispose) - { - value?.Dispose(); + var current = Interlocked.CompareExchange(ref _current, value, copy); + if (current == copy) + { + copy?.Dispose(); + return; + } + copy = current; } } } @@ -73,18 +77,7 @@ public IDisposable Disposable /// public void Dispose() { - var old = default(IDisposable); - - lock (_gate) - { - if (!_disposed) - { - _disposed = true; - old = _current; - _current = null; - } - } - + var old = Interlocked.Exchange(ref _current, BooleanDisposable.True); old?.Dispose(); } }