Inherit from NotificationObject to avoid memory leak in data binding#2890
Merged
ke-yu merged 1 commit intoDynamoDS:masterfrom Oct 30, 2014
Merged
Inherit from NotificationObject to avoid memory leak in data binding#2890ke-yu merged 1 commit intoDynamoDS:masterfrom
ke-yu merged 1 commit intoDynamoDS:masterfrom
Conversation
2 tasks
Contributor
|
The |
Contributor
|
I even remember reading that article, but it was hard to parse exactly what it meant. Basically any object that ever participates in data binding needs to implement INotifyPropertyChanged (we use NotificationObject). I know for a fact that we do exactly what is shown in that article in a number of places. I think we're safe with our nodes, ports, connectors and workspaces (anything inheriting from ModelBase). We'll just have to be super diligent in the future about not reintroducing this. Is there an automated memory test that we can add to track this? LGTM. |
ke-yu
added a commit
that referenced
this pull request
Oct 30, 2014
Inherit from NotificationObject to avoid memory leak in data binding
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request is part of attempt to fix memory leak on Revit.
Related pull request:
Looking at some retention paths, notice because some objects are referenced from system object,
RevitDynamoModelis not released (see images attached at the end of comment).These objects looks innocent, but according to this article, memory could easily be leak due to data binding. The solution is "Anything involved in a databinding should implement
INotifyPropertyChanged" (:-()I didn't dig too deep. But inheriting from
NotificationObjectremoves these objects onRevitDynamoModel's reference retention path.RevitDynamoModelis still referenced by other objects, but locally I've fixed it. I'll send the fix in other pull request.ShortCutBarItem
StartPageListItem