Skip to content

Eagerly dispose of Remotes #1284

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

Merged
merged 1 commit into from
Mar 23, 2016
Merged

Eagerly dispose of Remotes #1284

merged 1 commit into from
Mar 23, 2016

Conversation

niik
Copy link
Contributor

@niik niik commented Mar 19, 2016

This is a continuation of #1249. Since Remotes are now disposable we should eagerly clean up their resources rather than wait for GC to call
the destructor.

cc @ethomson @carlosmn

@niik
Copy link
Contributor Author

niik commented Mar 19, 2016

See #1249 (comment)

@niik
Copy link
Contributor Author

niik commented Mar 19, 2016

There's also a load of tests which use Remote that I didn't touch here

@niik niik force-pushed the dispose-of-remotes branch 2 times, most recently from 1bd0905 to b452a84 Compare March 19, 2016 23:45
@carlosmn
Copy link
Member

We could add

diff --git a/LibGit2Sharp/Remote.cs b/LibGit2Sharp/Remote.cs
index f92b4aa..a318707 100644
--- a/LibGit2Sharp/Remote.cs
+++ b/LibGit2Sharp/Remote.cs
@@ -39,6 +39,7 @@ internal Remote(RemoteSafeHandle handle, Repository repository)
             PushUrl = Proxy.git_remote_pushurl(handle);
             TagFetchMode = Proxy.git_remote_autotag(handle);
             refSpecs = new RefSpecCollection(this, handle);
+            repository.RegisterForCleanup(this);
         }

         ~Remote()

which makes us release the handle when the repository itself gets disposed.

This is a continuation of #1249. Since Remotes are now disposable we
should eagerly clean up their resources rather than wait for GC to call
the destructor.
@niik niik force-pushed the dispose-of-remotes branch from b452a84 to 7c23331 Compare March 23, 2016 02:25
@carlosmn carlosmn merged commit 89ce631 into master Mar 23, 2016
@bording bording deleted the dispose-of-remotes branch April 21, 2019 02:00
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