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

add dispose method for generated enum wrapper class DYN-5506 #13975

Merged
merged 3 commits into from
May 12, 2023

Conversation

mjkkirschner
Copy link
Member

@mjkkirschner mjkkirschner commented May 10, 2023

Purpose

https://jira.autodesk.com/browse/DYN-5506

@aparajit-pratap noticed an issue when creating and undoing creation of an enum backed ds object in dynamo in a codeblock.
I noticed that the generated wrapper class for each enum did not include a default dispose function which all generated type backed classes have - adding this removes the pointer to the enum wrapper from the dsobjectmap and clrobjectmap - which avoids exceptions from being thrown when a clr object is first bound to a dswrapper.

TODO

  • tests?

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Fixes a bug where enum backed dsobjects are not disposed fully.

Reviewers

@mjkkirschner mjkkirschner changed the title add dispose method for generated enum wrapper class add dispose method for generated enum wrapper class DYN-5506 May 10, 2023
@mjkkirschner
Copy link
Member Author

the test failures appear to be more flukes that we have seen before with parallel tests.

Comment on lines 157 to 162
[Test]
public void DisposeMultipleDispoableObject()
{
string code =
@"import(""FFITarget.dll"");tracer1 = HiddenDisposeTracer();tracer2 = HiddenDisposeTracer();count1 = tracer1.DisposeCount;count2 = tracer2.DisposeCount;[Associative]{ disposer1a = tracer1.GetHiddenDisposer(); disposer1a = null; disposer1b = tracer1.GetHiddenDisposer(); disposer1b = null; disposer2a = tracer2.GetHiddenDisposer(); disposer2a = null; disposer2b = tracer2.GetHiddenDisposer(); disposer2b = null; disposer2c = tracer2.GetHiddenDisposer(); disposer2c = null;}__GC();d1 = tracer1.DisposeCount;d2 = tracer2.DisposeCount;
";
thisTest.RunScriptSource(code);
thisTest.Verify("count1", 0);
thisTest.Verify("count2", 0);

thisTest.Verify("d1", 2);
thisTest.Verify("d2", 3); } //Migrate this code into the test framework private Subtree CreateSubTreeFromCode(Guid guid, string code) { var cbn = ProtoCore.Utils.ParserUtils.Parse(code); var subtree = null == cbn ? new Subtree(null, guid) : new Subtree(cbn.Body, guid); return subtree; } private void AssertValue(string varname, object value) { var mirror = astLiveRunner.InspectNodeValue(varname); MirrorData data = mirror.GetData(); object svValue = data.Data; if (value is double) { Assert.AreEqual((double)svValue, Convert.ToDouble(value)); } else if (value is int) { Assert.AreEqual(Convert.ToInt64(svValue), Convert.ToInt64(value)); } else if (value is bool) { Assert.AreEqual((bool)svValue, Convert.ToBoolean(value)); } } }} No newline at end of file
[Test] public void DisposeMultipleDispoableObject() { string code =@"import(""FFITarget.dll"");tracer1 = HiddenDisposeTracer();tracer2 = HiddenDisposeTracer();count1 = tracer1.DisposeCount;count2 = tracer2.DisposeCount;[Associative]{ disposer1a = tracer1.GetHiddenDisposer(); disposer1a = null; disposer1b = tracer1.GetHiddenDisposer(); disposer1b = null; disposer2a = tracer2.GetHiddenDisposer(); disposer2a = null; disposer2b = tracer2.GetHiddenDisposer(); disposer2b = null; disposer2c = tracer2.GetHiddenDisposer(); disposer2c = null;}__GC();d1 = tracer1.DisposeCount;d2 = tracer2.DisposeCount;
"; thisTest.RunScriptSource(code); thisTest.Verify("count1", 0); thisTest.Verify("count2", 0); thisTest.Verify("d1", 2); thisTest.Verify("d2", 3); } [Test] public void DisposeEnumWrapper() { string code = @"import(""FFITarget.dll"");x = Days.Monday;
__GC();x = Days.Monday;
__GC();
x = Days.Monday;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This diff is quite hard to read, I wonder if it's got to do with line endings or something?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow - I didn't realize it was that bad, let me take a look to see if I can improve it.

@mjkkirschner
Copy link
Member Author

I've made the diff worse ;) , but I think it will be better going forward as the line endings are now consistent for the entire file.

@mjkkirschner mjkkirschner merged commit 27c9a6a into DynamoDS:master May 12, 2023
@mjkkirschner mjkkirschner deleted the ztenum_err branch May 12, 2023 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants