Fix crashes by undoing/redoing nodes deletion

src/DynamoCore/Scheduler/UpdateGraphAsyncTask.cs

@@ -157,7 +157,7 @@ protected override void HandleTaskCompletionCore()
         /// </summary>
         /// <param name="other"></param>
         /// <returns></returns>
-        private bool Contains(UpdateGraphAsyncTask other)
+        private bool CanReplace(UpdateGraphAsyncTask other)
         {
             // Be more conservative here. Not only check ModifiedNodes, but
             // also check *all* nodes in graph sync data. For example, consider
@@ -175,9 +175,18 @@ private bool Contains(UpdateGraphAsyncTask other)
             else if (other.graphSyncData == null)
                 return true;
 
-            return other.graphSyncData.AddedNodeIDs.All(graphSyncData.AddedNodeIDs.Contains) &&
+            // Merging additions and removals can make the changeSetComputer internal state get corrupted.
+            // Imagine a sequence of queued tasks with changes like these: +A | -A | +AB
+            // Imagine a new task with change -A, ending up with the following sequence: -A | +A | -A | +AB
+            // While the merge routine would simplify this sequence to: -A | +A | +AB
+            // That creates an inconsistent sequence of changes where subtree A is added when it already exists!
+            // Modifications are not susceptible to this problem. Imagine ~A incoming with sequence: +A | -A | ~A | +AB
+            // The resulting sequence is: ~A | +A | -A | +AB, which is valid.
+            // Because modifications do not create or remove entries in the changeSetComputer internal state,
+            // if we remove one from anywhere in the sequence we remain consistent.
+            return other.graphSyncData.AddedNodeIDs.Count() == 0 &&
                    other.graphSyncData.ModifiedNodeIDs.All(graphSyncData.ModifiedNodeIDs.Contains) &&
-                   other.graphSyncData.DeletedNodeIDs.All(graphSyncData.DeletedNodeIDs.Contains);
+                   other.graphSyncData.DeletedNodeIDs.Count() == 0;
         }
 
         private bool IsScheduledAfter(UpdateGraphAsyncTask other)
@@ -188,12 +197,13 @@ private bool IsScheduledAfter(UpdateGraphAsyncTask other)
         protected override TaskMergeInstruction CanMergeWithCore(AsyncTask otherTask)
         {
             var theOtherTask = otherTask as UpdateGraphAsyncTask;
+
             if (theOtherTask == null)
                 return base.CanMergeWithCore(otherTask);
 
-            if (theOtherTask.IsScheduledAfter(this) && theOtherTask.Contains(this))
+            if (theOtherTask.IsScheduledAfter(this) && theOtherTask.CanReplace(this))
                 return TaskMergeInstruction.KeepOther;
-            else if (this.IsScheduledAfter(theOtherTask) && this.Contains(theOtherTask))
+            else if (this.IsScheduledAfter(theOtherTask) && this.CanReplace(theOtherTask))
                 return TaskMergeInstruction.KeepThis;
             else
                 return TaskMergeInstruction.KeepBoth;

src/DynamoCoreWpf/ViewModels/Core/HomeWorkspaceViewModel.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.ComponentModel;
 using System.Globalization;
 using System.Linq;
@@ -91,10 +92,27 @@ public override void OnNodeModified(NodeModel obj)
 
         private void hwm_SetNodeDeltaState(object sender, DeltaComputeStateEventArgs e)
         {
-            var nodeGuids = e.NodeGuidList;
+            // Using Nodes here is not thread safe, as nodes can be added/removed by the UI thread midway.
+            // Dispatching this to the UI thread helps avoid concurrency issues and seems more correct.
+            if (!DynamoViewModel.Model.ShutdownRequested)
+            {
+                DynamoViewModel.UIDispatcher.Invoke(() => UpdateNodesDeltaState(e.NodeGuidList, e.GraphExecuted));
+            }
+            else
+            {
+                // However, when Dynamo is shutting down, a funny deadlock situation occurs.
+                // The main thread is waiting for the scheduler to finish executing.
+                // So in order to avoid it, we do the update on the scheduler thread.
+                // Concurrency should not be an issue now that the UI is blocked waiting for shutdown.
+                UpdateNodesDeltaState(e.NodeGuidList, e.GraphExecuted);
+            }
+        }
+
+        private void UpdateNodesDeltaState(List<Guid> nodeGuids, bool graphExecuted)
+        {
             // if runsettings is manual, and if the graph is not executed, then turing on showrunpreview 
             //should turn on showexectionpreview on every node.
-            if (nodeGuids.Count == 0 && !e.GraphExecuted)
+            if (nodeGuids.Count == 0 && !graphExecuted)
             {
                 foreach (var nodeModel in Nodes)
                 {
@@ -104,7 +122,7 @@ private void hwm_SetNodeDeltaState(object sender, DeltaComputeStateEventArgs e)
 
             //if the graph is executed then set the node preview to false , provided
             // there is no error on that node.
-            if (nodeGuids.Count == 0 && e.GraphExecuted)
+            if (nodeGuids.Count == 0 && graphExecuted)
             {
                 foreach (var nodeViewModel in Nodes)
                 {
@@ -113,7 +131,7 @@ private void hwm_SetNodeDeltaState(object sender, DeltaComputeStateEventArgs e)
                         nodeViewModel.ShowExecutionPreview = false;
                         nodeViewModel.IsNodeAddedRecently = false;
                     }
-                }                
+                }
             }
 
             foreach (Guid t in nodeGuids)
@@ -132,7 +150,7 @@ private void hwm_SetNodeDeltaState(object sender, DeltaComputeStateEventArgs e)
             {
                 if (nodes.ShowExecutionPreview)
                     nodes.ShowExecutionPreview = nodes.DynamoViewModel.ShowRunPreview;
-            }           
+            }
         }
 
         void hwm_EvaluationCompleted(object sender, EvaluationCompletedEventArgs e)

src/Libraries/CoreNodeModelsWpf/Controls/DynamoSlider.xaml

@@ -73,7 +73,7 @@
                             Maximum="{Binding Max}" 
                             Minimum="{Binding Min}" 
                             TickFrequency="{Binding Step}" 
-                            Value="{Binding Value}" 
+                            Value="{Binding Value, Delay=500}" 
                             TickPlacement="None" 
                             IsSnapToTickEnabled="true" 
                             Grid.Column="2"