Skip to content

Updated InternalRegistration Set method to update node if there is an existing node - #175

Merged
ENikS merged 1 commit into
unitycontainer:v5.xfrom
danielp37:InterceptionMemoryLeakFix
Dec 4, 2019
Merged

ENikS merged 1 commit into
unitycontainer:v5.xfrom
danielp37:InterceptionMemoryLeakFix

Conversation

@danielp37

Copy link
Copy Markdown

Updated InternalRegistration Set method to update node if there is an existing node
to update node if there is an existing node with the key instead of always inserting. The
always inserting functionality was causing a memory leak with the
Unity.Interception because the PreBuildUp was always inserting a new
node on every resolve.

I'm not sure that this is the correct fix given the current design but I did verify that it prevents the current memory leak.

an existing node with the key instead of always inserting.  The
always inserting functionality was causing a memory leak with the
Unity.Interception because the PreBuildUp was always inserting a new
node on every resolve.
@ENikS

ENikS commented Nov 30, 2019

Copy link
Copy Markdown
Contributor

The idea behind this logic is to insert a new policy before the old one and avoid looping. Next time policy is requested it is discovered first and returned instead of old one.

@danielp37

Copy link
Copy Markdown
Author

Ok, the problem (I think, it has been a few months since I've looked at this) was that it was inserting a new node on every resolve of the interface and the new node was exactly like the previous one. I don't think that this was intended. And even though it should discover this one first, there were other policies that were at the end of the linked-list that would have to be searched and so after a while the list would become long causing a memory leak and it would have to search the entire linked list causing more CPU to be used.

@danielp37

Copy link
Copy Markdown
Author

The problem might be with Interception in that it is inserting a new node on each resolve but I don't understand enough of it currently to understand why it would be doing this on each resolve.

@ENikS
ENikS merged commit 7665c67 into unitycontainer:v5.x Dec 4, 2019
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