Handle runtime table gaps on code block deletion (#10605)

mmisol · web-flow · commit 81df8a93517f · 2020-05-04T08:55:24.000-04:00
When the runtime table are built there is an implicit assumption that
the code block ids are consecutive. However, that is not always the
case, as the deletion of a procedure causes the deletion of its child
code blocks, which may generate gaps in the id numbering.

In order to make the code resiliant to these gaps, the runtime tables
are sized based on the largest code block id, rather than in the amount
of code blocks.
diff --git a/src/Engine/ProtoCore/Core.cs b/src/Engine/ProtoCore/Core.cs
@@ -727,7 +727,7 @@ public void GenerateExecutable()
             // The TypeSystem is a record of all primitive and compiler generated types
             DSExecutable.TypeSystem = TypeSystem;
 
-            RuntimeTableIndex = CompleteCodeBlockList.Count;
+            RuntimeTableIndex = GetRuntimeTableSize();
 
 
             // Build the runtime symbols
@@ -762,6 +762,30 @@ public void GenerateExecutable()
             DSExecutable.CurrentDSFileName = CurrentDSFileName;           
         }
 
+        /// <summary>
+        /// Gets the size to be used for runtime tables of symbols, procedures and instruction streams.
+        /// Note: since blocks are stored consecutively but may have gaps due to procedures being deleted,
+        /// this is based on largest id rather than amount of blocks.
+        /// </summary>
+        internal int GetRuntimeTableSize()
+        {
+            // Due to the way this list is constructed, the largest id is the one of the last block.
+            var lastBlock = CompleteCodeBlockList.LastOrDefault();
+            // If there are no code blocks yet, then the required size for tables is 0.
+            // This happens when the first code block is being created and its id is being generated.
+            if (lastBlock == null)
+            {
+                return 0;
+            }
+
+            // If the last block has children, then its last child has the largest id.
+            if (lastBlock.children.Count > 0)
+            {
+                lastBlock = lastBlock.children.Last();
+            }
+            return lastBlock.codeBlockId + 1;
+        }
+
         public string GenerateTempVar()
         {
             tempVarId++;
diff --git a/src/Engine/ProtoCore/DSASM/Executable.cs b/src/Engine/ProtoCore/DSASM/Executable.cs
@@ -155,10 +155,10 @@ public CodeBlock(Guid guid, CodeBlockType type, ProtoCore.Language langId, int c
             procedureTable = procTable;
 
             isBreakable = isBreakableBlock;
+            codeBlockId = core.GetRuntimeTableSize();
             core.CompleteCodeBlockList.Add(this);
-            this.codeBlockId = core.CompleteCodeBlockList.Count - 1;
 
-            symbols.RuntimeIndex = this.codeBlockId;
+            symbols.RuntimeIndex = codeBlockId;
 
             if (core.ProcNode != null)
             {
diff --git a/test/DynamoCoreTests/ImperativeClodeBlockTest.cs b/test/DynamoCoreTests/ImperativeClodeBlockTest.cs
@@ -1,5 +1,8 @@
 ﻿using System.Collections.Generic;
+using System.Linq;
+using Dynamo.Scheduler;
 using NUnit.Framework;
+using static Dynamo.Models.DynamoModel;
 
 namespace Dynamo.Tests
 {
@@ -22,5 +25,29 @@ public void TestCallStaticFunction()
             RunModel(@"core\imperative\call_static_function.dyn");
             AssertPreviewValue("250880ed-5d34-49e7-b92b-2a7f9336f62b", new object[] { 3, 5 });
         }
+
+        [Test]
+        public void DeletingImperativeCBNShouldNotLeadToCrash()
+        {
+            TaskStateChangedEventHandler evaluationDidNotFailHandler =
+                (DynamoScheduler sender, TaskStateChangedEventArgs args) =>
+                {
+                    Assert.AreNotEqual(TaskStateChangedEventArgs.State.ExecutionFailed, args.CurrentState);
+                };
+            try
+            {
+                CurrentDynamoModel.Scheduler.TaskStateChanged += evaluationDidNotFailHandler;
+                OpenModel(@"core\imperative\delete_imperative_cbn_crash.dyn");
+                AssertPreviewValue("0692b19256834a9187f3bcd500d513f1", 5.86);
+                CurrentDynamoModel.ExecuteCommand(new DeleteModelCommand("45f0db0f-c014-481e-9b7f-939da54f2adc"));
+                CurrentDynamoModel.ExecuteCommand(new DeleteModelCommand("235f53ba-d913-45d0-986a-b9c6588cba45"));
+                AssertPreviewValue("0692b19256834a9187f3bcd500d513f1", 5.86);
+                Assert.AreEqual(2, CurrentDynamoModel.CurrentWorkspace.Nodes.Count());
+            }
+            finally
+            {
+                CurrentDynamoModel.Scheduler.TaskStateChanged -= evaluationDidNotFailHandler;
+            }
+        }
     }
 }
diff --git a/test/core/imperative/delete_imperative_cbn_crash.dyn b/test/core/imperative/delete_imperative_cbn_crash.dyn
@@ -0,0 +1,174 @@
+{
+  "Uuid": "5cbd5e74-0c40-4731-8de5-a14376da2743",
+  "IsCustomNode": false,
+  "Description": null,
+  "Name": "crash_repro",
+  "ElementResolver": {
+    "ResolutionMap": {
+      "Curve.StartPoint": {
+        "Key": "Autodesk.DesignScript.Geometry.Curve",
+        "Value": "ProtoGeometry.dll"
+      },
+      "Curve.EndPoint": {
+        "Key": "Autodesk.DesignScript.Geometry.Curve",
+        "Value": "ProtoGeometry.dll"
+      },
+      "Math": {
+        "Key": "DSCore.Math",
+        "Value": "DSCoreNodes.dll"
+      }
+    }
+  },
+  "Inputs": [],
+  "Outputs": [],
+  "Nodes": [
+    {
+      "ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
+      "NodeType": "CodeBlockNode",
+      "Code": "def giveMeEPlusPi()\n{\n  impPi = [Imperative]\n  {\n    return giveMePi();\n  }\n  impE = [Imperative]\n  {\n    return giveMeE();\n  }\n  return Math.Sum([impPi, impE]);\n}\n\ndef giveMeE()\n{\n  return 2.718;\n}\n\ndef giveMePi()\n{\n  return 3.142;\n};",
+      "Id": "a531f0bb1b8b42418b58a5676e2b0107",
+      "Inputs": [],
+      "Outputs": [],
+      "Replication": "Disabled",
+      "Description": "Allows for DesignScript code to be authored directly"
+    },
+    {
+      "ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
+      "NodeType": "CodeBlockNode",
+      "Code": "a=giveMeEPlusPi();",
+      "Id": "0692b19256834a9187f3bcd500d513f1",
+      "Inputs": [],
+      "Outputs": [
+        {
+          "Id": "6867de1c60b84ac5944f76aea5f0520a",
+          "Name": "",
+          "Description": "a",
+          "UsingDefaultValue": false,
+          "Level": 2,
+          "UseLevels": false,
+          "KeepListStructure": false
+        }
+      ],
+      "Replication": "Disabled",
+      "Description": "Allows for DesignScript code to be authored directly"
+    },
+    {
+      "ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
+      "NodeType": "CodeBlockNode",
+      "Code": "b=a+1;",
+      "Id": "235f53bad91345d0986ab9c6588cba45",
+      "Inputs": [
+        {
+          "Id": "44f93bd8548141c9a8f2f7338c925e21",
+          "Name": "a",
+          "Description": "a",
+          "UsingDefaultValue": false,
+          "Level": 2,
+          "UseLevels": false,
+          "KeepListStructure": false
+        }
+      ],
+      "Outputs": [
+        {
+          "Id": "db93f82ac4874eff840022e9abeb5731",
+          "Name": "",
+          "Description": "b",
+          "UsingDefaultValue": false,
+          "Level": 2,
+          "UseLevels": false,
+          "KeepListStructure": false
+        }
+      ],
+      "Replication": "Disabled",
+      "Description": "Allows for DesignScript code to be authored directly"
+    },
+    {
+      "ConcreteType": "Dynamo.Graph.Nodes.CodeBlockNodeModel, DynamoCore",
+      "NodeType": "CodeBlockNode",
+      "Code": "def grpCrvs(Curve:var[]..[])\n    {\n    \tdis1 = (Curve.StartPoint)<1>.DistanceTo(Curve<2>)==0\n        ||(Curve.EndPoint)<1>.DistanceTo(Curve<2>)==0;\n    \tind1 = DSCore.List.AllIndicesOf(dis1<1>,true);\n    \tbln1 = true;\n    \tind2 = [Imperative]\n    \t{\n    \t\twhile (bln1)\n    \t\t{\n    \t\t\tcnt1 = DSCore.List.Count(ind1);\n    \t\t\tind1 = grpIndx(ind1);\n    \t\t\tcnt2 = DSCore.List.Count(ind1);\n    \t\t\tbln1 = cnt2!=cnt1;\n    \t\t}\n    \t\treturn = ind1;\n    \t}\n    \tcrv1 = DSCore.List.GetItemAtIndex(Curve,ind2);\n    \treturn = crv1;\n    };\n\n    def grpIndx(ind:var[]..[])\n    {\n    \tind1 = DSCore.List.SetIntersection(ind<1>,ind<2>);\n    \tcnt1 = DSCore.List.Count(ind1<1><2>)>1;\n    \tind2 = DSCore.List.FilterByBoolMask(ind,cnt1<1>)[\"in\"];\n    \tind3 = DSCore.List.UniqueItems(DSCore.List.Flatten(ind2<1>,-1)<1>);\n    \tind4 = DSCore.List.UniqueItems(DSCore.List.Sort(ind3<1>));\n    \treturn = ind4;\n    };",
+      "Id": "45f0db0fc014481e9b7f939da54f2adc",
+      "Inputs": [],
+      "Outputs": [],
+      "Replication": "Disabled",
+      "Description": "Allows for DesignScript code to be authored directly"
+    }
+  ],
+  "Connectors": [
+    {
+      "Start": "6867de1c60b84ac5944f76aea5f0520a",
+      "End": "44f93bd8548141c9a8f2f7338c925e21",
+      "Id": "27eebd44113640e7a1f161caeadac75e"
+    }
+  ],
+  "Dependencies": [],
+  "NodeLibraryDependencies": [],
+  "Bindings": [],
+  "View": {
+    "Dynamo": {
+      "ScaleFactor": 1.0,
+      "HasRunWithoutCrash": true,
+      "IsVisibleInDynamoLibrary": true,
+      "Version": "2.7.0.8475",
+      "RunType": "Automatic",
+      "RunPeriod": "1000"
+    },
+    "Camera": {
+      "Name": "Background Preview",
+      "EyeX": -17.0,
+      "EyeY": 24.0,
+      "EyeZ": 50.0,
+      "LookX": 12.0,
+      "LookY": -13.0,
+      "LookZ": -58.0,
+      "UpX": 0.0,
+      "UpY": 1.0,
+      "UpZ": 0.0
+    },
+    "NodeViews": [
+      {
+        "ShowGeometry": true,
+        "Name": "Code Block",
+        "Id": "a531f0bb1b8b42418b58a5676e2b0107",
+        "IsSetAsInput": false,
+        "IsSetAsOutput": false,
+        "Excluded": false,
+        "X": 554.2,
+        "Y": 95.0
+      },
+      {
+        "ShowGeometry": true,
+        "Name": "Code Block",
+        "Id": "0692b19256834a9187f3bcd500d513f1",
+        "IsSetAsInput": false,
+        "IsSetAsOutput": false,
+        "Excluded": false,
+        "X": -19.0,
+        "Y": 26.0
+      },
+      {
+        "ShowGeometry": true,
+        "Name": "Code Block",
+        "Id": "235f53bad91345d0986ab9c6588cba45",
+        "IsSetAsInput": false,
+        "IsSetAsOutput": false,
+        "Excluded": false,
+        "X": 213.83216955952196,
+        "Y": 28.548083791714419
+      },
+      {
+        "ShowGeometry": true,
+        "Name": "Code Block",
+        "Id": "45f0db0fc014481e9b7f939da54f2adc",
+        "IsSetAsInput": false,
+        "IsSetAsOutput": false,
+        "Excluded": false,
+        "X": -113.913086330586,
+        "Y": 147.03575124246467
+      }
+    ],
+    "Annotations": [],
+    "X": 83.554627092476267,
+    "Y": 60.75454555821301,
+    "Zoom": 0.68246059654089
+  }
+}
