Fixes the underlying LinkedList retain cycle issue

Tyler-Keith-Thompson commented 3 years ago

…Had to comment out sort tests because something very strange is happening with it - TT

Linked Issue: Closes #38

Checklist:

[x] Is the linter reporting 0 errors?
[x] Did you comply with our styleguide?
[x] Is there adequate test coverage for your new code?
[x] Does the CI pipeline pass?
[x] Is the static analysis tool reporting 0 issues?
[x] Did you update the documentation?
[ ] Did you update the sample app?
[ ] Do we need to increment the minor/major version?
[ ] Did you change the public API?
[ ] Have you done everything you can to make sure that errors that can occur are compile-time errors, and if they have to be runtime do you have adequate tests and documentation around those runtime errors? For more details.

If Applicable:

[ ] Did you test when the first item is skipped?
[ ] Did you test when the last item is skipped?
[ ] Did you test when middle items are skipped?
[ ] Did you test when incorrect data is passed forward?
[ ] Did you test proceeding backwards?

If Public API Has Changed:

[ ] Did you deprecate (rather than remove) any old methods/variables/etc? Our philosophy for deprecation.
[ ] Have you done the best that you can to make sure that the compiler guides people to changing to the new API? (Example: the renamed attribute)
[ ] If necessary, have you tested the upgrade path for at least N-1 versions? For example, if data persists between v1 and v2 then that upgrade should be tested and as easy as we can make it.

Tyler-Keith-Thompson commented 3 years ago

I have functioning code that resolves the memory leak, but it has caused unexpected behavior with sort. I thought I'd create a draft pull request in case the team decided that sorting was sort of a silly operation to even allow on a Workflow anyways and wanted to just delete the sort code and move on.

I will continue to work on fixing that in my spare time.

For those curious where the memory leak was, it's just that the LinkedList needed a deinit to clean up strong references on nodes. Nothing major, I've got a test around that.

Tyler-Keith-Thompson commented 3 years ago

I withdraw my last statement, that sorting issue exposed a much more serious one with how removeTillEnd worked. Can't believe that was missed. It's now all fixed, and we're much better about memory usage AND we don't have an infinitely looping function in the codebase anymore. Yay!

sonarcloud[bot] commented 3 years ago

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

100.0% Coverage
0.0% Duplication

wwt / SwiftCurrent