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

Closing a repository document leaks everything #439

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Closing a repository document leaks everything #439

wants to merge 3 commits into from

Conversation

mattlilek
Copy link

Closing a repository's window doesn't return any memory to the system. It looks like there's retain cycles everywhere.

This branch doesn't yet eliminate all leaks but fixes some big ones.

…er 'super controller',

and the only place these objects are ever created is within the PBGitWindowController...which
contains a strong, owning reference to these objects creating a retain cycle.
…ts by breaking a retain

cycle between the PBGitHistoryList and its internal PBGitHistoryGrapher. The grapher maintained
a strong reference to it's "delegate", but is only ever owned by the same object.

Make the "delegate" a weak reference. The two places it's used, create a strong reference while it's used.

Finally, -[PBGitRepository close] calls -[PBGitHistoryList cleanup] but so does -[PBGitHistoryList dealloc].
Avoid an uncaught exception/crash by nil-ing out the currentRevList after we've cleaned up as it's no
longer valid.
means many more objects are getting released at the proper time.
@tiennou
Copy link

tiennou commented Mar 18, 2015

Side note, make sure that you're not weak-ing things that can't be weak — we have been bit by NSViewController not being allowed to be weak on 10.8 in think.

@5sw
Copy link

5sw commented Jun 28, 2015

I already fixed all those leaks and cycles about two years ago: #224

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.

3 participants