Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle runtime table gaps on code block deletion #10605

Merged
merged 8 commits into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion src/Engine/ProtoCore/Core.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -762,6 +762,17 @@ 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>
private int GetRuntimeTableSize()
QilongTang marked this conversation as resolved.
Show resolved Hide resolved
{
// Due to the way this list is constructed, the largest id is the one of the last block.
return CompleteCodeBlockList.Last().codeBlockId + 1;
}

public string GenerateTempVar()
{
tempVarId++;
Expand Down
27 changes: 27 additions & 0 deletions test/DynamoCoreTests/ImperativeClodeBlockTest.cs
Original file line number Diff line number Diff line change
@@ -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
{
Expand All @@ -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;
}
}
}
}
174 changes: 174 additions & 0 deletions test/core/imperative/delete_imperative_cbn_crash.dyn
Original file line number Diff line number Diff line change
@@ -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
}
}