RE-Pm publishpackage packagecontents

[dnenov]

Purpose

This PR #14496 has been reverted due to a potential memory leak. (All tests passing, but it is crashing the live test runner).

WIP, still investigating the memory leak.

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

Reviewers

@QilongTang
@mjkkirschner
@reddyashish

FYIs

@Amoursol

[mjkkirschner]

@dnenov what makes you conclude that a memory leak is crashing the test runner?

[QilongTang]

@dnenov what makes you conclude that a memory leak is crashing the test runner?

Hey @mjkkirschner I think I was asking Deyan to look at potential memory leaks, any other clues? I did run the tests against this branch and tests still crashing in the middle:

Failed! - Failed: 1, Passed: 986, Skipped: 5, Total: 992, Duration: 46 m 43 s - DynamoCoreWpfTests.dll (net6.0)
Test Run Aborted with error System.Exception: One or more errors occurred.
---> System.Exception: Unable to read data from the transport connection: An existing connection was forcibly closed by the remote host..
---> System.Exception: An existing connection was forcibly closed by the remote host.
at System.Net.Sockets.NetworkStream.Read(Span1 buffer) --- End of inner exception stack trace --- at System.Net.Sockets.NetworkStream.Read(Span1 buffer)
at System.Net.Sockets.NetworkStream.ReadByte()
at System.IO.BinaryReader.Read7BitEncodedInt()
at System.IO.BinaryReader.ReadString()
at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.LengthPrefixCommunicationChannel.NotifyDataAvailable()
at Microsoft.VisualStudio.TestPlatform.CommunicationUtilities.TcpClientExtensions.MessageLoopAsync(TcpClient client, ICommunicationChannel channel, Action`1 errorHandler, CancellationToken cancellationToken)
--- End of inner exception stack trace ---.

[dnenov]

@dnenov what makes you conclude that a memory leak is crashing the test runner?

Sorry for the delayed response @mjkkirschner , I was trying to get as much information as I can before reporting back.
I did an experiment where I disposed of all ViewModels as well as the PackageManagerView once the view was closed. I did it so I could see if all the resources were collected, and they were not.
I also followed the final example from this article creating finalizers in some of the main view models and forcing garbage collection to see if the finalizers were called, and again they were not.

This is a snapshot of the memory allocation after:

1. Open Dynamo
2. Open and close the PackageManager (substantial memory increase)
3. Open and close the PackageManager for the second time (minimal memory increase, but still some)

This is a snapshot of the Heap after the PackageManager is closed:

There seems to be a list of Greg.Responses.PackageVersion that takes up the bulk of the space. Also, the PackageManagerView is still there.

I went through the code paying particular attention to unsubscribing from handlers, and I think I haven't missed anything. Let's see if this makes any difference.

[mjkkirschner]

Personally, I don't think it's a memory leak - I think you are encountering the same issue as @pinzart90 and I where the web browser is being interacted with on the wrong thread due to the nature of the dispatcher and tests using the dispatcher.

To see if you have a memory leak that might affect the tests, just run the all the tests and watch your memory. The test machines might have about 16 to 64 gigs of ram.

You'll probably see that the test runner crashes way before you run out of memory.

[dnenov]

Personally, I don't think it's a memory leak - I think you are encountering the same issue as @pinzart90 and I where the web browser is being interacted with on the wrong thread due to the nature of the dispatcher and tests using the dispatcher.

To see if you have a memory leak that might affect the tests, just run the all the tests and watch your memory. The test machines might have about 16 to 64 gigs of ram.

You'll probably see that the test runner crashes way before you run out of memory.

Yes, I hope so at least. I went through all possible event handlers I could find and am sure we are unsubscribing correctly. If all fails, I will break down the PR into smaller chunks and see if we can narrow down the moment it starts failing.

Thank you for the input @mjkkirschner !!