Ensure Singleton Closes Any Opened Object During Concurrent Open/Close (

#174)

* Check if disposed after calling OnCreateAsync()

* Verify singleton handles concurrent close/open

src/Fx/Singleton.cs

@@ -88,7 +88,7 @@ protected virtual void Dispose(bool disposing)
             {
                 this.disposed = true;
                 var thisTaskCompletionSource = this.taskCompletionSource;
-                if (thisTaskCompletionSource != null && thisTaskCompletionSource.Task.Status == TaskStatus.RanToCompletion)
+                if (thisTaskCompletionSource != null && thisTaskCompletionSource.Task.Status == TaskStatus.RanToCompletion && this.TryRemove())
                 {
                     OnSafeClose(thisTaskCompletionSource.Task.Result);
                 }
@@ -126,6 +126,11 @@ public async Task<TValue> GetOrCreateAsync(TimeSpan timeout)
                     {
                         TValue value = await this.OnCreateAsync(timeout).ConfigureAwait(false);
                         tcs.SetResult(value);
+
+                        if (this.disposed && this.TryRemove())
+                        {
+                          OnSafeClose(value);
+                        }
                     }
                     catch (Exception ex) when (!Fx.IsFatal(ex))
                     {

test/Test.Microsoft.Amqp/TestCases/SingletonTests.cs

@@ -0,0 +1,63 @@
+using System;
+using System.Threading.Tasks;
+using Microsoft.Azure.Amqp;
+using Xunit;
+
+namespace Test.Microsoft.Azure.Amqp
+{
+    public class SingletonTests
+    {
+        [Fact]
+        public async Task SingletonConcurrentCloseOpenTests()
+        {
+            var createTcs = new TaskCompletionSource<object>();
+            var closeTcs = new TaskCompletionSource<object>();
+
+            var singleton = new SingletonTester(createTcs.Task, closeTcs.Task);
+
+            var creating = singleton.GetOrCreateAsync(TimeSpan.FromSeconds(1));
+            var closing = singleton.CloseAsync();
+
+            closeTcs.SetResult(new object());
+            await closing;
+
+            createTcs.SetResult(new object());
+            await creating;
+
+            await Assert.ThrowsAsync<ObjectDisposedException>(() => singleton.GetOrCreateAsync(TimeSpan.FromSeconds(1)));
+
+            var createdObj = GetInternalProperty<object>(singleton, "Value");
+            Assert.Null(createdObj);
+        }
+
+        private class SingletonTester : Singleton<object>
+        {
+            private readonly Task<object> _onCreateComplete;
+            private readonly Task<object> _onSafeCloseComplete;
+
+            public SingletonTester(Task<object> onCreateComplete, Task<object> onSafeCloseComplete)
+            {
+                _onCreateComplete = onCreateComplete;
+                _onSafeCloseComplete = onSafeCloseComplete;
+            }
+
+            protected override async Task<object> OnCreateAsync(TimeSpan timeout)
+            {
+                await _onCreateComplete;
+                return new object();
+            }
+
+            protected override void OnSafeClose(object value)
+            {
+                _onSafeCloseComplete.GetAwaiter().GetResult();
+            }
+        }
+
+        private T GetInternalProperty<T>(object from, string propertyName)
+          where T : class
+        {
+            var prop = from.GetType().GetProperty(propertyName, System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance);
+            return prop.GetValue(from) as T;
+        }
+    }
+}