Skip to content
This repository was archived by the owner on Dec 5, 2024. It is now read-only.

Commit befcbf7

Browse files
authored
Merge pull request #615 from github-for-unity/1.0rc/fixes/dispose-of-things
Make sure events are unhooked when we're disposing so they don't leak
2 parents 9ea8923 + 187662d commit befcbf7

File tree

9 files changed

+75
-14
lines changed

9 files changed

+75
-14
lines changed

src/GitHub.Api/Cache/CacheContainer.cs

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ public void InvalidateAll()
3737

3838
private IManagedCache SetupCache(IManagedCache cache)
3939
{
40-
cache.CacheInvalidated += () => OnCacheInvalidated(cache.CacheType);
41-
cache.CacheUpdated += datetime => OnCacheUpdated(cache.CacheType, datetime);
40+
cache.CacheInvalidated += OnCacheInvalidated;
41+
cache.CacheUpdated += OnCacheUpdated;
4242
return cache;
4343
}
4444

@@ -68,6 +68,28 @@ private void OnCacheInvalidated(CacheType cacheType)
6868
CacheInvalidated.SafeInvoke(cacheType);
6969
}
7070

71+
private bool disposed;
72+
private void Dispose(bool disposing)
73+
{
74+
if (disposed) return;
75+
disposed = true;
76+
77+
if (disposing)
78+
{
79+
foreach (var cache in caches.Values)
80+
{
81+
cache.Value.CacheInvalidated -= OnCacheInvalidated;
82+
cache.Value.CacheUpdated -= OnCacheUpdated;
83+
}
84+
}
85+
}
86+
87+
public void Dispose()
88+
{
89+
Dispose(true);
90+
GC.SuppressFinalize(this);
91+
}
92+
7193
public IBranchCache BranchCache { get { return (IBranchCache)caches[CacheType.Branches].Value; } }
7294
public IGitLogCache GitLogCache { get { return (IGitLogCache)caches[CacheType.GitLog].Value; } }
7395
public IGitAheadBehindCache GitTrackingStatusCache { get { return (IGitAheadBehindCache)caches[CacheType.GitAheadBehind].Value; } }

src/GitHub.Api/Cache/CacheInterfaces.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ public enum CacheType
1414
GitUser
1515
}
1616

17-
public interface ICacheContainer
17+
public interface ICacheContainer : IDisposable
1818
{
1919
event Action<CacheType> CacheInvalidated;
2020
event Action<CacheType, DateTimeOffset> CacheUpdated;
@@ -34,8 +34,8 @@ public interface ICacheContainer
3434

3535
public interface IManagedCache
3636
{
37-
event Action CacheInvalidated;
38-
event Action<DateTimeOffset> CacheUpdated;
37+
event Action<CacheType> CacheInvalidated;
38+
event Action<CacheType, DateTimeOffset> CacheUpdated;
3939

4040
bool ValidateData();
4141
void InvalidateData();

src/GitHub.Api/Events/RepositoryWatcher.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,14 @@ private void Dispose(bool disposing)
276276
if (!disposed)
277277
{
278278
disposed = true;
279+
HeadChanged = null;
280+
IndexChanged = null;
281+
ConfigChanged = null;
282+
RepositoryCommitted = null;
283+
RepositoryChanged = null;
284+
LocalBranchesChanged = null;
285+
RemoteBranchesChanged = null;
286+
279287
Stop();
280288
if (nativeInterface != null)
281289
{

src/GitHub.Api/Git/IRepository.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ namespace GitHub.Unity
66
/// <summary>
77
/// Represents a repository, either local or retrieved via the GitHub API.
88
/// </summary>
9-
public interface IRepository : IEquatable<IRepository>
9+
public interface IRepository : IEquatable<IRepository>, IDisposable
1010
{
1111
void Initialize(IRepositoryManager repositoryManager, ITaskManager taskManager);
1212
void Start();

src/GitHub.Api/Git/Repository.cs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,25 @@ private static GitBranch GetLocalGitBranch(string currentBranchName, ConfigBranc
282282
return new GitBranch(branchName, trackingName, isActive);
283283
}
284284

285+
286+
private bool disposed;
287+
private void Dispose(bool disposing)
288+
{
289+
if (disposing)
290+
{
291+
if (!disposed)
292+
{
293+
disposed = true;
294+
}
295+
}
296+
}
297+
298+
public void Dispose()
299+
{
300+
Dispose(true);
301+
}
302+
303+
285304
private static GitBranch GetRemoteGitBranch(ConfigBranch x) => new GitBranch(x.Remote.Value.Name + "/" + x.Name, "[None]", false);
286305
private static GitRemote GetGitRemote(ConfigRemote configRemote) => new GitRemote(configRemote.Name, configRemote.Url);
287306

src/GitHub.Api/Git/RepositoryManager.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ public interface IRepositoryManager : IDisposable
1818
event Action<Dictionary<string, ConfigBranch>> LocalBranchesUpdated;
1919
event Action<Dictionary<string, ConfigRemote>, Dictionary<string, Dictionary<string, ConfigBranch>>> RemoteBranchesUpdated;
2020
event Action<GitAheadBehindStatus> GitAheadBehindStatusUpdated;
21+
2122
event Action<CacheType> DataNeedsRefreshing;
2223

2324
void Initialize();
@@ -115,6 +116,7 @@ class RepositoryManager : IRepositoryManager
115116
public event Action<List<GitLogEntry>> GitLogUpdated;
116117
public event Action<Dictionary<string, ConfigBranch>> LocalBranchesUpdated;
117118
public event Action<Dictionary<string, ConfigRemote>, Dictionary<string, Dictionary<string, ConfigBranch>>> RemoteBranchesUpdated;
119+
118120
public event Action<CacheType> DataNeedsRefreshing;
119121

120122
public RepositoryManager(IGitConfig gitConfig,
@@ -610,6 +612,14 @@ private void Dispose(bool disposing)
610612

611613
if (disposing)
612614
{
615+
CurrentBranchUpdated = null;
616+
GitStatusUpdated = null;
617+
GitAheadBehindStatusUpdated = null;
618+
GitLogUpdated = null;
619+
GitLocksUpdated = null;
620+
LocalBranchesUpdated = null;
621+
RemoteBranchesUpdated = null;
622+
DataNeedsRefreshing = null;
613623
Stop();
614624
watcher.Dispose();
615625
}

src/UnityExtension/Assets/Editor/GitHub.Unity/ApplicationCache.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,8 @@ abstract class ManagedCacheBase<T> : ScriptObjectSingleton<T> where T : Scriptab
133133
[NonSerialized] private DateTimeOffset? lastUpdatedAtValue;
134134
[NonSerialized] private DateTimeOffset? initializedAtValue;
135135

136-
public event Action CacheInvalidated;
137-
public event Action<DateTimeOffset> CacheUpdated;
136+
public event Action<CacheType> CacheInvalidated;
137+
public event Action<CacheType, DateTimeOffset> CacheUpdated;
138138

139139
protected ManagedCacheBase(CacheType type)
140140
{
@@ -159,7 +159,7 @@ public void InvalidateData()
159159
{
160160
Logger.Trace("Invalidate");
161161
LastUpdatedAt = DateTimeOffset.MinValue;
162-
CacheInvalidated.SafeInvoke();
162+
CacheInvalidated.SafeInvoke(CacheType);
163163
}
164164

165165
protected void SaveData(DateTimeOffset now, bool isChanged)
@@ -175,7 +175,7 @@ protected void SaveData(DateTimeOffset now, bool isChanged)
175175
if (isChanged)
176176
{
177177
Logger.Trace("Updated: {0}", now);
178-
CacheUpdated.SafeInvoke(now);
178+
CacheUpdated.SafeInvoke(CacheType, now);
179179
}
180180
}
181181

src/tests/IntegrationTests/BaseIntegrationTest.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,8 @@ public virtual void OnSetup()
212212
public virtual void OnTearDown()
213213
{
214214
TaskManager.Dispose();
215+
Environment?.CacheContainer.Dispose();
216+
215217
Logger.Debug("Deleting TestBasePath: {0}", TestBasePath.ToString());
216218
for (var i = 0; i < 5; i++)
217219
{

src/tests/IntegrationTests/CachingClasses.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ abstract class ManagedCacheBase<T> : ScriptObjectSingleton<T> where T : class, I
109109

110110
private bool isInvalidating;
111111

112-
public event Action CacheInvalidated;
113-
public event Action<DateTimeOffset> CacheUpdated;
112+
public event Action<CacheType> CacheInvalidated;
113+
public event Action<CacheType, DateTimeOffset> CacheUpdated;
114114

115115
protected ManagedCacheBase(CacheType type)
116116
{
@@ -138,7 +138,7 @@ public void InvalidateData()
138138
{
139139
isInvalidating = true;
140140
LastUpdatedAt = DateTimeOffset.MinValue;
141-
CacheInvalidated.SafeInvoke();
141+
CacheInvalidated.SafeInvoke(CacheType);
142142
}
143143
}
144144

@@ -157,7 +157,7 @@ protected void SaveData(DateTimeOffset now, bool isChanged)
157157
if (isChanged)
158158
{
159159
Logger.Trace("Updated: {0}", now);
160-
CacheUpdated.SafeInvoke(now);
160+
CacheUpdated.SafeInvoke(CacheType, now);
161161
}
162162
}
163163

0 commit comments

Comments
 (0)