Make sure events are unhooked when we're disposing so they don't leak

Most of our objects are singletons that live through the lifetime of the process
(the lifetime being the time between Unity reloading domains whenever it recompiles
or the user enters play mode), we never really worry much about unhooking events
to allow objects to be GC'd and to not raise events on objects that are dead.
In our tests, however, that's a problem, since we're not reloading the domain and
killing all the things between one test and the other, which means events can
definitely leak and may cause problems.

This makes sure we break event loops so that tests don't trample all over each
other. It's not a thorough audit of everything that we need to dispose, but it's
a start.

Author: shana

src/GitHub.Api/Cache/CacheContainer.cs

@@ -37,8 +37,8 @@ public void InvalidateAll()
 
         private IManagedCache SetupCache(IManagedCache cache)
         {
-            cache.CacheInvalidated += () => OnCacheInvalidated(cache.CacheType);
-            cache.CacheUpdated += datetime => OnCacheUpdated(cache.CacheType, datetime);
+            cache.CacheInvalidated += OnCacheInvalidated;
+            cache.CacheUpdated += OnCacheUpdated;
             return cache;
         }
 
@@ -68,6 +68,28 @@ private void OnCacheInvalidated(CacheType cacheType)
             CacheInvalidated.SafeInvoke(cacheType);
         }
 
+        private bool disposed;
+        private void Dispose(bool disposing)
+        {
+            if (disposed) return;
+            disposed = true;
+
+            if (disposing)
+            {
+                foreach (var cache in caches.Values)
+                {
+                    cache.Value.CacheInvalidated -= OnCacheInvalidated;
+                    cache.Value.CacheUpdated -= OnCacheUpdated;
+                }
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+            GC.SuppressFinalize(this);
+        }
+
         public IBranchCache BranchCache { get { return (IBranchCache)caches[CacheType.Branches].Value; } }
         public IGitLogCache GitLogCache { get { return (IGitLogCache)caches[CacheType.GitLog].Value; } }
         public IGitAheadBehindCache GitTrackingStatusCache { get { return (IGitAheadBehindCache)caches[CacheType.GitAheadBehind].Value; } }

src/GitHub.Api/Cache/CacheInterfaces.cs

@@ -14,7 +14,7 @@ public enum CacheType
         GitUser
     }
 
-    public interface ICacheContainer
+    public interface ICacheContainer : IDisposable
     {
         event Action<CacheType> CacheInvalidated;
         event Action<CacheType, DateTimeOffset> CacheUpdated;
@@ -34,8 +34,8 @@ public interface ICacheContainer
 
     public interface IManagedCache
     {
-        event Action CacheInvalidated;
-        event Action<DateTimeOffset> CacheUpdated;
+        event Action<CacheType> CacheInvalidated;
+        event Action<CacheType, DateTimeOffset> CacheUpdated;
 
         bool ValidateData();
         void InvalidateData();

src/GitHub.Api/Events/RepositoryWatcher.cs

@@ -276,6 +276,14 @@ private void Dispose(bool disposing)
                 if (!disposed)
                 {
                     disposed = true;
+                    HeadChanged = null;
+                    IndexChanged = null;
+                    ConfigChanged = null;
+                    RepositoryCommitted = null;
+                    RepositoryChanged = null;
+                    LocalBranchesChanged = null;
+                    RemoteBranchesChanged = null;
+
                     Stop();
                     if (nativeInterface != null)
                     {

src/GitHub.Api/Git/IRepository.cs

@@ -6,7 +6,7 @@ namespace GitHub.Unity
     /// <summary>
     /// Represents a repository, either local or retrieved via the GitHub API.
     /// </summary>
-    public interface IRepository : IEquatable<IRepository>
+    public interface IRepository : IEquatable<IRepository>, IDisposable
     {
         void Initialize(IRepositoryManager repositoryManager, ITaskManager taskManager);
         void Start();

src/GitHub.Api/Git/Repository.cs

@@ -282,6 +282,25 @@ private static GitBranch GetLocalGitBranch(string currentBranchName, ConfigBranc
             return new GitBranch(branchName, trackingName, isActive);
         }
 
+
+        private bool disposed;
+        private void Dispose(bool disposing)
+        {
+            if (disposing)
+            {
+                if (!disposed)
+                {
+                    disposed = true;
+                }
+            }
+        }
+
+        public void Dispose()
+        {
+            Dispose(true);
+        }
+
+
         private static GitBranch GetRemoteGitBranch(ConfigBranch x) => new GitBranch(x.Remote.Value.Name + "/" + x.Name, "[None]", false);
         private static GitRemote GetGitRemote(ConfigRemote configRemote) => new GitRemote(configRemote.Name, configRemote.Url);
 

src/GitHub.Api/Git/RepositoryManager.cs

@@ -18,6 +18,7 @@ public interface IRepositoryManager : IDisposable
         event Action<Dictionary<string, ConfigBranch>> LocalBranchesUpdated;
         event Action<Dictionary<string, ConfigRemote>, Dictionary<string, Dictionary<string, ConfigBranch>>> RemoteBranchesUpdated;
         event Action<GitAheadBehindStatus> GitAheadBehindStatusUpdated;
+
         event Action<CacheType> DataNeedsRefreshing;
 
         void Initialize();
@@ -115,6 +116,7 @@ class RepositoryManager : IRepositoryManager
         public event Action<List<GitLogEntry>> GitLogUpdated;
         public event Action<Dictionary<string, ConfigBranch>> LocalBranchesUpdated;
         public event Action<Dictionary<string, ConfigRemote>, Dictionary<string, Dictionary<string, ConfigBranch>>> RemoteBranchesUpdated;
+
         public event Action<CacheType> DataNeedsRefreshing;
 
         public RepositoryManager(IGitConfig gitConfig,
@@ -610,6 +612,14 @@ private void Dispose(bool disposing)
 
             if (disposing)
             {
+                CurrentBranchUpdated = null;
+                GitStatusUpdated = null;
+                GitAheadBehindStatusUpdated = null;
+                GitLogUpdated = null;
+                GitLocksUpdated = null;
+                LocalBranchesUpdated = null;
+                RemoteBranchesUpdated = null;
+                DataNeedsRefreshing = null;
                 Stop();
                 watcher.Dispose();
             }

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

@@ -133,8 +133,8 @@ abstract class ManagedCacheBase<T> : ScriptObjectSingleton<T> where T : Scriptab
         [NonSerialized] private DateTimeOffset? lastUpdatedAtValue;
         [NonSerialized] private DateTimeOffset? initializedAtValue;
 
-        public event Action CacheInvalidated;
-        public event Action<DateTimeOffset> CacheUpdated;
+        public event Action<CacheType> CacheInvalidated;
+        public event Action<CacheType, DateTimeOffset> CacheUpdated;
 
         protected ManagedCacheBase(CacheType type)
         {
@@ -159,7 +159,7 @@ public void InvalidateData()
         {
             Logger.Trace("Invalidate");
             LastUpdatedAt = DateTimeOffset.MinValue;
-            CacheInvalidated.SafeInvoke();
+            CacheInvalidated.SafeInvoke(CacheType);
         }
 
         protected void SaveData(DateTimeOffset now, bool isChanged)
@@ -175,7 +175,7 @@ protected void SaveData(DateTimeOffset now, bool isChanged)
             if (isChanged)
             {
                 Logger.Trace("Updated: {0}", now);
-                CacheUpdated.SafeInvoke(now);
+                CacheUpdated.SafeInvoke(CacheType, now);
             }
         }
 

src/tests/IntegrationTests/BaseIntegrationTest.cs

@@ -212,6 +212,8 @@ public virtual void OnSetup()
         public virtual void OnTearDown()
         {
             TaskManager.Dispose();
+            Environment?.CacheContainer.Dispose();
+
             Logger.Debug("Deleting TestBasePath: {0}", TestBasePath.ToString());
             for (var i = 0; i < 5; i++)
             {

src/tests/IntegrationTests/CachingClasses.cs

@@ -109,8 +109,8 @@ abstract class ManagedCacheBase<T> : ScriptObjectSingleton<T> where T : class, I
 
         private bool isInvalidating;
 
-        public event Action CacheInvalidated;
-        public event Action<DateTimeOffset> CacheUpdated;
+        public event Action<CacheType> CacheInvalidated;
+        public event Action<CacheType, DateTimeOffset> CacheUpdated;
 
         protected ManagedCacheBase(CacheType type)
         {
@@ -138,7 +138,7 @@ public void InvalidateData()
             {
                 isInvalidating = true;
                 LastUpdatedAt = DateTimeOffset.MinValue;
-                CacheInvalidated.SafeInvoke();
+                CacheInvalidated.SafeInvoke(CacheType);
             }
         }
 
@@ -157,7 +157,7 @@ protected void SaveData(DateTimeOffset now, bool isChanged)
             if (isChanged)
             {
                 Logger.Trace("Updated: {0}", now);
-                CacheUpdated.SafeInvoke(now);
+                CacheUpdated.SafeInvoke(CacheType, now);
             }
         }