Skip to content

Commit cc00a21

Browse files
mminnsmminns
committed
Issue-1471 Fix handle leaks
1 parent 26db5b9 commit cc00a21

File tree

7 files changed

+84
-53
lines changed

7 files changed

+84
-53
lines changed

‎LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
<PropertyGroup>
44
<TargetFrameworks>net461;netcoreapp2.0</TargetFrameworks>
5-
<DefineConstants Condition=" '$(TargetFramework)' == 'net461' ">$(DefineConstants);DESKTOP</DefineConstants>
5+
<DefineConstants Condition=" '$(TargetFramework)' == 'net461' ">TRACE;DEBUG;DESKTOP;NET461;LEAKS_IDENTIFYING;LEAKS_TRACKING</DefineConstants>
66
</PropertyGroup>
77

88
<ItemGroup>

‎LibGit2Sharp.Tests/WorktreeFixture.cs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,10 @@ public void CanGetRepositoryForWorktree()
128128
var worktree = repo.Worktrees["logo"];
129129

130130
Assert.Equal("logo", worktree.Name);
131-
var worktreeRepo = worktree.WorktreeRepository;
132-
Assert.NotNull(worktreeRepo);
131+
using (var repository = worktree.WorktreeRepository)
132+
{
133+
Assert.NotNull(repository);
134+
}
133135
}
134136
}
135137

@@ -259,7 +261,10 @@ public void CanAddWorktreeForCommittish()
259261
var worktree = repo.Worktrees.Add(committish, name, path, false);
260262
Assert.Equal(name, worktree.Name);
261263
Assert.False(worktree.IsLocked);
262-
Assert.Equal(committish, worktree.WorktreeRepository.Head.FriendlyName);
264+
using (var repository = worktree.WorktreeRepository)
265+
{
266+
Assert.Equal(committish, repository.Head.FriendlyName);
267+
}
263268
Assert.Equal(3, repo.Worktrees.Count());
264269
}
265270
}

‎LibGit2Sharp/Core/Handles/Libgit2Object.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ internal IntPtr AsIntPtr()
138138

139139
void Dispose(bool disposing)
140140
{
141-
#if LEAKS_IDENTIFYING
141+
#if LEAKS_IDENTIFYING
142142
bool leaked = !disposing && ptr != null;
143143

144144
if (leaked)

‎LibGit2Sharp/LibGit2Sharp.csproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616
<AssemblyOriginatorKeyFile>..\libgit2sharp.snk</AssemblyOriginatorKeyFile>
1717
</PropertyGroup>
1818

19+
<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">
20+
<DefineConstants>TRACE;DEBUG;NETSTANDARD2_0;LEAKS_IDENTIFYING;LEAKS_TRACKING</DefineConstants>
21+
</PropertyGroup>
22+
1923
<ItemGroup>
2024
<CodeAnalysisDictionary Include="CustomDictionary.xml" />
2125
<None Include="Core\Handles\Objects.tt">

‎LibGit2Sharp/Repository.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ internal Repository(WorktreeHandle worktreeHandle)
9999
{
100100
handle = Proxy.git_repository_open_from_worktree(worktreeHandle);
101101
RegisterForCleanup(handle);
102+
RegisterForCleanup(worktreeHandle);
102103

103104
isBare = Proxy.git_repository_is_bare(handle);
104105

‎LibGit2Sharp/Worktree.cs

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,8 @@ public class Worktree : IEquatable<Worktree>, IBelongToARepository
1717
private static readonly LambdaEqualityHelper<Worktree> equalityHelper =
1818
new LambdaEqualityHelper<Worktree>(x => x.Name);
1919

20-
private readonly WorktreeHandle handle;
2120
private readonly Repository parent;
22-
private readonly Repository worktree;
21+
//private readonly Repository worktree;
2322
private readonly string name;
2423
private WorktreeLock worktreeLock;
2524

@@ -29,15 +28,22 @@ public class Worktree : IEquatable<Worktree>, IBelongToARepository
2928
protected Worktree()
3029
{ }
3130

32-
internal Worktree(WorktreeHandle handle, Repository repo, string name, Repository worktree, WorktreeLock worktreeLock)
31+
internal Worktree(Repository repo, string name, WorktreeLock worktreeLock)
3332
{
34-
this.handle = Proxy.git_worktree_lookup(repo.Handle, name);
3533
this.parent = repo;
3634
this.name = name;
37-
this.worktree = worktree;
3835
this.worktreeLock = worktreeLock;
3936
}
4037

38+
/// <summary>
39+
///
40+
/// </summary>
41+
/// <returns></returns>
42+
internal WorktreeHandle GetWorktreeHandle()
43+
{
44+
return Proxy.git_worktree_lookup(parent.Handle, name);
45+
}
46+
4147
/// <summary>
4248
/// The name of the worktree.
4349
/// </summary>
@@ -46,7 +52,7 @@ internal Worktree(WorktreeHandle handle, Repository repo, string name, Repositor
4652
/// <summary>
4753
/// The Repository representation of the worktree
4854
/// </summary>
49-
public virtual Repository WorktreeRepository { get { return worktree; } }
55+
public virtual Repository WorktreeRepository { get { return new Repository(GetWorktreeHandle()); } }
5056

5157
/// <summary>
5258
/// A flag indicating if the worktree is locked or not.
@@ -83,17 +89,23 @@ public bool Equals(Worktree other)
8389
/// </summary>
8490
public virtual void Unlock()
8591
{
86-
Proxy.git_worktree_unlock(handle);
87-
this.worktreeLock = Proxy.git_worktree_is_locked(handle);
92+
using (var handle = GetWorktreeHandle())
93+
{
94+
Proxy.git_worktree_unlock(handle);
95+
this.worktreeLock = Proxy.git_worktree_is_locked(handle);
96+
}
8897
}
8998

9099
/// <summary>
91100
/// Lock the worktree
92101
/// </summary>
93102
public virtual void Lock(string reason)
94103
{
95-
Proxy.git_worktree_lock(handle, reason);
96-
this.worktreeLock = Proxy.git_worktree_is_locked(handle);
104+
using (var handle = GetWorktreeHandle())
105+
{
106+
Proxy.git_worktree_lock(handle, reason);
107+
this.worktreeLock = Proxy.git_worktree_is_locked(handle);
108+
}
97109
}
98110

99111
/// <summary>
@@ -123,7 +135,5 @@ private string DebuggerDisplay
123135
}
124136

125137
IRepository IBelongToARepository.Repository { get { return parent; } }
126-
127-
internal WorktreeHandle Handle { get { return this.handle; } }
128138
}
129139
}

‎LibGit2Sharp/WorktreeCollection.cs

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,8 @@ public virtual Worktree this[string name]
4141
{
4242
Ensure.ArgumentNotNullOrEmptyString(name, "name");
4343

44-
return Lookup(name, handle => new Worktree(handle, repo,
44+
return Lookup(name, handle => new Worktree(repo,
4545
name,
46-
new Repository(handle),
4746
Proxy.git_worktree_is_locked(handle)));
4847
}
4948
}
@@ -70,15 +69,21 @@ public virtual Worktree Add(string committishOrBranchSpec, string name, string p
7069
locked = Convert.ToInt32(isLocked)
7170
};
7271

73-
var handle = Proxy.git_worktree_add(repo.Handle, name, path, options);
74-
var worktree = new Worktree(handle,
75-
repo,
76-
name,
77-
new Repository(handle),
78-
Proxy.git_worktree_is_locked(handle));
72+
using (var handle = Proxy.git_worktree_add(repo.Handle, name, path, options))
73+
{
74+
var worktree = new Worktree(
75+
repo,
76+
name,
77+
Proxy.git_worktree_is_locked(handle));
78+
79+
// switch the worktree to the target branch
80+
using (var repository = worktree.WorktreeRepository)
81+
{
82+
Commands.Checkout(repository, committishOrBranchSpec);
83+
}
84+
}
7985

80-
// switch the worktree to the target branch
81-
Commands.Checkout(worktree.WorktreeRepository, committishOrBranchSpec);
86+
8287

8388
return this[name];
8489
}
@@ -97,12 +102,13 @@ public virtual Worktree Add(string name, string path, bool isLocked)
97102
locked = Convert.ToInt32(isLocked)
98103
};
99104

100-
var handle = Proxy.git_worktree_add(repo.Handle, name, path, options);
101-
return new Worktree(handle,
102-
repo,
103-
name,
104-
new Repository(handle),
105-
Proxy.git_worktree_is_locked(handle));
105+
using (var handle = Proxy.git_worktree_add(repo.Handle, name, path, options))
106+
{
107+
return new Worktree(
108+
repo,
109+
name,
110+
Proxy.git_worktree_is_locked(handle));
111+
}
106112
}
107113

108114
/// <summary>
@@ -123,26 +129,32 @@ public virtual bool Prune(Worktree worktree)
123129
/// <returns></returns>
124130
public virtual bool Prune(Worktree worktree, bool ifLocked)
125131
{
126-
string wd = worktree.WorktreeRepository.Info.WorkingDirectory;
127-
128-
if (!Directory.Exists(wd))
129-
{
130-
return false;
131-
}
132-
133-
git_worktree_prune_options options = new git_worktree_prune_options
132+
using (var handle = worktree.GetWorktreeHandle())
134133
{
135-
version = 1,
136-
// default
137-
flags = GitWorktreePruneOptionFlags.GIT_WORKTREE_PRUNE_WORKING_TREE | GitWorktreePruneOptionFlags.GIT_WORKTREE_PRUNE_VALID
138-
};
139-
140-
if (ifLocked)
141-
{
142-
options.flags |= GitWorktreePruneOptionFlags.GIT_WORKTREE_PRUNE_LOCKED;
134+
using (var repository = worktree.WorktreeRepository)
135+
{
136+
string wd = repository.Info.WorkingDirectory;
137+
138+
if (!Directory.Exists(wd))
139+
{
140+
return false;
141+
}
142+
143+
git_worktree_prune_options options = new git_worktree_prune_options
144+
{
145+
version = 1,
146+
// default
147+
flags = GitWorktreePruneOptionFlags.GIT_WORKTREE_PRUNE_WORKING_TREE | GitWorktreePruneOptionFlags.GIT_WORKTREE_PRUNE_VALID
148+
};
149+
150+
if (ifLocked)
151+
{
152+
options.flags |= GitWorktreePruneOptionFlags.GIT_WORKTREE_PRUNE_LOCKED;
153+
}
154+
155+
return Proxy.git_worktree_prune(handle, options);
156+
}
143157
}
144-
145-
return Proxy.git_worktree_prune(worktree.Handle, options);
146158
}
147159

148160
internal T Lookup<T>(string name, Func<WorktreeHandle, T> selector, bool throwIfNotFound = false)
@@ -170,8 +182,7 @@ internal T Lookup<T>(string name, Func<WorktreeHandle, T> selector, bool throwIf
170182
public virtual IEnumerator<Worktree> GetEnumerator()
171183
{
172184
return Proxy.git_worktree_list(repo.Handle)
173-
.Select(n => Lookup(n, handle => new Worktree(handle, repo, n,
174-
new Repository(handle), Proxy.git_worktree_is_locked(handle))))
185+
.Select(n => Lookup(n, handle => new Worktree(repo, n, Proxy.git_worktree_is_locked(handle))))
175186
.GetEnumerator();
176187
}
177188

0 commit comments

Comments
 (0)