Debugger: Clean up on Disconnection and Identifier Thread Safety (#3629)

* move definition closer to use

* fix continued event

* add "break on start" to launch and attach arguments

* add item and more to source Range in protocol

* cache ICodePoint.Data to minimize new identitier values

* cache IReadOnlyList<ICodePoint> Frames to minimize new identifier values

* move files to folders for sanity

* add JValue data model

* continue all threads when disconnecting

* factor out interface from Identifier<T>

* add fixed size identifier cache

* factor out arena value codes from thread model

* add OutputEvent variable reference

* include value type in scalar data model

* fixes after testing

* reset debugger state on disconnection

* make identifier cache thread safe

* bad merge

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/DebuggerSourceMap.cs

@@ -88,6 +88,16 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
             }
         }
 
+        void IBreakpoints.Clear()
+        {
+            lock (gate)
+            {
+                this.rows.Clear();
+                this.items.Clear();
+                this.dirty = true;
+            }
+        }
+
         IReadOnlyList<Protocol.Breakpoint> IBreakpoints.SetBreakpoints(Protocol.Source source, IReadOnlyList<Protocol.SourceBreakpoint> sourceBreakpoints)
         {
             lock (gate)

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/DialogDebugAdapter.cs

@@ -29,10 +29,10 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
 
         // lifetime scoped to IMiddleware.OnTurnAsync
         private readonly ConcurrentDictionary<string, ThreadModel> threadByTurnId = new ConcurrentDictionary<string, ThreadModel>();
-        private readonly IIdentifier<ThreadModel> threads = new Identifier<ThreadModel>();
+        private readonly IIdentifier<ThreadModel> threads = new Identifier<ThreadModel>().WithMutex();
 
         // https://en.wikipedia.org/wiki/Region-based_memory_management
-        private readonly IIdentifier<ArenaModel> arenas = new Identifier<ArenaModel>();
+        private readonly IIdentifier<ArenaModel> arenas = new Identifier<ArenaModel>().WithMutex();
         private readonly OutputModel output = new OutputModel();
 
         private readonly Task task;
@@ -270,7 +270,7 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
                 {
                     this.Logger.LogError(error, error.Message);
 
-                    this.ContinueAllThreads();
+                    this.ResetOnDisconnect();
 
                     throw;
                 }
@@ -315,6 +315,16 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
             frame = thread.FrameCodes[valueCode];
         }
 
+        private void ResetOnDisconnect()
+        {
+            // consider resetting this.events filter enabled state to defaults from constructor
+
+            this.options = new Protocol.LaunchAttach();
+            this.breakpoints.Clear();
+            this.output.ValueCodes.Clear();
+            ContinueAllThreads();
+        }
+
         private void ContinueAllThreads()
         {
             var errors = new List<Exception>();
@@ -647,7 +657,7 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
                 }
                 else
                 {
-                    this.ContinueAllThreads();
+                    this.ResetOnDisconnect();
                 }
 
                 return Protocol.Response.From(NextSeq, disconnect, new { });
@@ -702,7 +712,7 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
         private sealed class OutputModel : ArenaModel
         {
             public OutputModel()
-                : base(new IdentifierCache<object>(new Identifier<object>(), count: 25))
+                : base(new Identifier<object>().WithCache(count: 25).WithMutex())
             {
             }
         }
@@ -710,7 +720,7 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
         private sealed class ThreadModel : ArenaModel
         {
             public ThreadModel(ITurnContext turnContext, ICodeModel codeModel)
-                : base(new Identifier<object>())
+                : base(new Identifier<object>().WithMutex())
             {
                 TurnContext = turnContext;
                 CodeModel = codeModel;
@@ -740,7 +750,7 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
 
             public RunModel Run { get; } = new RunModel();
 
-            public IIdentifier<ICodePoint> FrameCodes { get; } = new Identifier<ICodePoint>();
+            public IIdentifier<ICodePoint> FrameCodes { get; } = new Identifier<ICodePoint>().WithMutex();
 
             public DialogContext LastContext { get; private set; }
 

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/IBreakpoints.cs

@@ -12,6 +12,8 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
 
         object ItemFor(Protocol.Breakpoint breakpoint);
 
+        void Clear();
+
         IReadOnlyList<Protocol.Breakpoint> SetBreakpoints(Protocol.Source source, IReadOnlyList<Protocol.SourceBreakpoint> sourceBreakpoints);
 
         IReadOnlyList<Protocol.Breakpoint> SetBreakpoints(IReadOnlyList<Protocol.FunctionBreakpoint> functionBreakpoints);

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/Identifiers/IIdentifier{T}.cs

@@ -25,6 +25,8 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
 
         bool TryGetValue(T item, out ulong code);
 
+        void Clear();
+
         ulong Add(T item);
 
         void Remove(T item);

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/Identifiers/IdentifierCache.cs

@@ -28,6 +28,12 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
 
         bool IIdentifier<T>.TryGetValue(T item, out ulong code) => this.inner.TryGetValue(item, out code);
 
+        void IIdentifier<T>.Clear()
+        {
+            this.inner.Clear();
+            this.queue.Clear();
+        }
+
         ulong IIdentifier<T>.Add(T item)
         {
             if (this.inner.TryGetValue(item, out var code))

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/Identifiers/IdentifierFactory.cs

@@ -0,0 +1,15 @@
+using System;
+using System.Collections.Generic;
+using System.Text;
+
+namespace Microsoft.Bot.Builder.Dialogs.Debugging
+{
+    public static class IdentifierFactory
+    {
+        public static IIdentifier<T> WithCache<T>(this IIdentifier<T> identifier, int count)
+            => new IdentifierCache<T>(identifier, count);
+
+        public static IIdentifier<T> WithMutex<T>(this IIdentifier<T> identifier)
+            => new IdentifierMutex<T>(identifier);
+    }
+}

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/Identifiers/IdentifierMutex.cs

@@ -0,0 +1,108 @@
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+namespace Microsoft.Bot.Builder.Dialogs.Debugging
+{
+    public sealed class IdentifierMutex<T> : IIdentifier<T>
+    {
+        private readonly IIdentifier<T> inner;
+        private readonly object gate = new object();
+
+        public IdentifierMutex(IIdentifier<T> inner)
+        {
+            this.inner = inner ?? throw new ArgumentNullException(nameof(inner));
+        }
+
+        IEnumerable<T> IIdentifier<T>.Items
+        {
+            get
+            {
+                lock (this.gate)
+                {
+                    return this.inner.Items.ToList();
+                }
+            }
+        }
+
+        T IIdentifier<T>.this[ulong code]
+        {
+            get
+            {
+                lock (this.gate)
+                {
+                    return this.inner[code];
+                }
+            }
+        }
+
+        ulong IIdentifier<T>.this[T item]
+        {
+            get
+            {
+                lock (this.gate)
+                {
+                    return this.inner[item];
+                }
+            }
+        }
+
+        bool IIdentifier<T>.TryGetValue(ulong code, out T item)
+        {
+            lock (this.gate)
+            {
+                return this.inner.TryGetValue(code, out item);
+            }
+        }
+
+        bool IIdentifier<T>.TryGetValue(T item, out ulong code)
+        {
+            lock (this.gate)
+            {
+                return this.inner.TryGetValue(item, out code);
+            }
+        }
+
+        ulong IIdentifier<T>.Add(T item)
+        {
+            lock (this.gate)
+            {
+                return this.inner.Add(item);
+            }
+        }
+
+        void IIdentifier<T>.Remove(T item)
+        {
+            lock (this.gate)
+            {
+                this.inner.Remove(item);
+            }
+        }
+
+        void IIdentifier<T>.Clear()
+        {
+            lock (this.gate)
+            {
+                this.inner.Clear();
+            }
+        }
+
+        IEnumerator<KeyValuePair<ulong, T>> IEnumerable<KeyValuePair<ulong, T>>.GetEnumerator()
+        {
+            lock (this.gate)
+            {
+                return this.inner.ToList().GetEnumerator();
+            }
+        }
+
+        IEnumerator IEnumerable.GetEnumerator()
+        {
+            lock (this.gate)
+            {
+                return this.inner.ToList().GetEnumerator();
+            }
+        }
+    }
+}

libraries/Microsoft.Bot.Builder.Dialogs.Debugging/Identifiers/Identifier{T}.cs

@@ -21,92 +21,54 @@ namespace Microsoft.Bot.Builder.Dialogs.Debugging
     {
         private readonly Dictionary<T, ulong> codeByItem = new Dictionary<T, ulong>(ReferenceEquality<T>.Instance);
         private readonly Dictionary<ulong, T> itemByCode = new Dictionary<ulong, T>();
-        private readonly object gate = new object();
         private ulong last = 0;
 
         IEnumerable<T> IIdentifier<T>.Items
-        {
-            get
-            {
-                lock (gate)
-                {
-                    return this.itemByCode.Values.ToArray();
-                }
-            }
-        }
+            => this.itemByCode.Values.ToArray();
 
         T IIdentifier<T>.this[ulong code]
-        {
-            get
-            {
-                lock (gate)
-                {
-                    return this.itemByCode[code];
-                }
-            }
-        }
+            => this.itemByCode[code];
 
         ulong IIdentifier<T>.this[T item]
-        {
-            get
-            {
-                lock (gate)
-                {
-                    return this.codeByItem[item];
-                }
-            }
-        }
+            => this.codeByItem[item];
 
         bool IIdentifier<T>.TryGetValue(ulong code, out T item)
-        {
-            lock (gate)
-            {
-                return this.itemByCode.TryGetValue(code, out item);
-            }
-        }
+            => this.itemByCode.TryGetValue(code, out item);
 
         bool IIdentifier<T>.TryGetValue(T item, out ulong code)
+            => this.codeByItem.TryGetValue(item, out code);
+
+        void IIdentifier<T>.Clear()
         {
-            lock (gate)
-            {
-                return this.codeByItem.TryGetValue(item, out code);
-            }
+            // do not reset the last code to avoid any risk of https://en.wikipedia.org/wiki/ABA_problem
+            this.itemByCode.Clear();
+            this.codeByItem.Clear();
         }
 
         ulong IIdentifier<T>.Add(T item)
         {
-            lock (gate)
+            if (!this.codeByItem.TryGetValue(item, out var code))
             {
-                if (!this.codeByItem.TryGetValue(item, out var code))
-                {
-                    // avoid false values
-                    code = ++last;
-                    this.codeByItem.Add(item, code);
-                    this.itemByCode.Add(code, item);
-                }
-
-                return code;
+                // avoid false values
+                code = ++last;
+                this.codeByItem.Add(item, code);
+                this.itemByCode.Add(code, item);
             }
+
+            return code;
         }
 
         void IIdentifier<T>.Remove(T item)
         {
-            lock (gate)
-            {
-                var code = this.codeByItem[item];
-                this.itemByCode.Remove(code);
-                this.codeByItem.Remove(item);
-            }
+            var code = this.codeByItem[item];
+            this.itemByCode.Remove(code);
+            this.codeByItem.Remove(item);
         }
 
         IEnumerator<KeyValuePair<ulong, T>> IEnumerable<KeyValuePair<ulong, T>>.GetEnumerator()
-        {
-            lock (gate)
-            {
-                return this.itemByCode.ToList().GetEnumerator();
-            }
-        }
+            => this.itemByCode.GetEnumerator();
 
-        IEnumerator IEnumerable.GetEnumerator() => ((IEnumerable<KeyValuePair<ulong, T>>)this).GetEnumerator();
+        IEnumerator IEnumerable.GetEnumerator()
+            => this.itemByCode.GetEnumerator();
     }
 }