Resolve FilterFixture NRE

LibGit2Sharp.Tests/FilterFixture.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.IO;
 using LibGit2Sharp.Tests.TestHelpers;
 using Xunit;
@@ -26,7 +27,7 @@ public void CanRegisterFilterWithSingleAttribute()
         [Fact]
         public void CanRegisterAndUnregisterTheSameFilter()
         {
-            var filter = new EmptyFilter(FilterName + 1, attributes);
+            var filter = new EmptyFilter(FilterName, attributes);
 
             var registration = GlobalSettings.RegisterFilter(filter);
             GlobalSettings.DeregisterFilter(registration);
@@ -38,8 +39,7 @@ public void CanRegisterAndUnregisterTheSameFilter()
         [Fact]
         public void CanRegisterAndDeregisterAfterGarbageCollection()
         {
-            var filter = new EmptyFilter(FilterName + 2, attributes);
-            var filterRegistration = GlobalSettings.RegisterFilter(filter);
+            var filterRegistration = GlobalSettings.RegisterFilter(new EmptyFilter(FilterName, attributes));
 
             GC.Collect();
 
@@ -49,7 +49,7 @@ public void CanRegisterAndDeregisterAfterGarbageCollection()
         [Fact]
         public void SameFilterIsEqual()
         {
-            var filter = new EmptyFilter(FilterName + 3, attributes);
+            var filter = new EmptyFilter(FilterName, attributes);
             Assert.Equal(filter, filter);
         }
 
@@ -62,16 +62,16 @@ public void InitCallbackNotMadeWhenFilterNeverUsed()
                 called = true;
             };
 
-            var filter = new FakeFilter(FilterName + 11, attributes,
-                successCallback,
-                successCallback,
-                initializeCallback);
-
-            var filterRegistration = GlobalSettings.RegisterFilter(filter);
+            var filter = new FakeFilter(FilterName,
+                                        attributes,
+                                        successCallback,
+                                        successCallback,
+                                        initializeCallback);
+            var registration = GlobalSettings.RegisterFilter(filter);
 
             Assert.False(called);
 
-            GlobalSettings.DeregisterFilter(filterRegistration);
+            GlobalSettings.DeregisterFilter(registration);
         }
 
         [Fact]
@@ -83,12 +83,12 @@ public void InitCallbackMadeWhenUsingTheFilter()
                 called = true;
             };
 
-            var filter = new FakeFilter(FilterName + 12, attributes,
-                successCallback,
-                successCallback,
-                initializeCallback);
-
-            var filterRegistration = GlobalSettings.RegisterFilter(filter);
+            var filter = new FakeFilter(FilterName,
+                                        attributes,
+                                        successCallback,
+                                        successCallback,
+                                        initializeCallback);
+            var registration = GlobalSettings.RegisterFilter(filter);
             Assert.False(called);
 
             string repoPath = InitNewRepository();
@@ -98,7 +98,7 @@ public void InitCallbackMadeWhenUsingTheFilter()
                 Assert.True(called);
             }
 
-            GlobalSettings.DeregisterFilter(filterRegistration);
+            GlobalSettings.DeregisterFilter(registration);
         }
 
         [Fact]
@@ -112,17 +112,17 @@ public void WhenStagingFileApplyIsCalledWithCleanForCorrectPath()
                 called = true;
                 reader.CopyTo(writer);
             };
-            var filter = new FakeFilter(FilterName + 15, attributes, clean);
 
-            var filterRegistration = GlobalSettings.RegisterFilter(filter);
+            var filter = new FakeFilter(FilterName, attributes, clean);
+            var registration = GlobalSettings.RegisterFilter(filter);
 
             using (var repo = CreateTestRepository(repoPath))
             {
                 StageNewFile(repo);
                 Assert.True(called);
             }
 
-            GlobalSettings.DeregisterFilter(filterRegistration);
+            GlobalSettings.DeregisterFilter(registration);
         }
 
         [Fact]
@@ -135,22 +135,20 @@ public void CleanFilterWritesOutputToObjectTree()
 
             Action<Stream, Stream> cleanCallback = SubstitutionCipherFilter.RotateByThirteenPlaces;
 
-            var filter = new FakeFilter(FilterName + 16, attributes, cleanCallback);
-
-            var filterRegistration = GlobalSettings.RegisterFilter(filter);
+            var filter = new FakeFilter(FilterName, attributes, cleanCallback);
+            var registration = GlobalSettings.RegisterFilter(filter);
 
             using (var repo = CreateTestRepository(repoPath))
             {
                 FileInfo expectedFile = StageNewFile(repo, decodedInput);
                 var commit = repo.Commit("Clean that file");
-
                 var blob = (Blob)commit.Tree[expectedFile.Name].Target;
 
                 var textDetected = blob.GetContentText();
                 Assert.Equal(encodedInput, textDetected);
             }
 
-            GlobalSettings.DeregisterFilter(filterRegistration);
+            GlobalSettings.DeregisterFilter(registration);
         }
 
         [Fact]
@@ -164,16 +162,16 @@ public void WhenCheckingOutAFileFileSmudgeWritesCorrectFileToWorkingDirectory()
 
             Action<Stream, Stream> smudgeCallback = SubstitutionCipherFilter.RotateByThirteenPlaces;
 
-            var filter = new FakeFilter(FilterName + 17, attributes, null, smudgeCallback);
-            var filterRegistration = GlobalSettings.RegisterFilter(filter);
+            var filter = new FakeFilter(FilterName, attributes, null, smudgeCallback);
+            var registration = GlobalSettings.RegisterFilter(filter);
 
             FileInfo expectedFile = CheckoutFileForSmudge(repoPath, branchName, encodedInput);
 
             string combine = Path.Combine(repoPath, "..", expectedFile.Name);
             string readAllText = File.ReadAllText(combine);
             Assert.Equal(decodedInput, readAllText);
 
-            GlobalSettings.DeregisterFilter(filterRegistration);
+            GlobalSettings.DeregisterFilter(registration);
         }
 
         [Fact]
@@ -186,8 +184,8 @@ public void CanFilterLargeFiles()
 
             string repoPath = InitNewRepository();
 
-            var filter = new FileExportFilter("exportFilter", attributes);
-            var filterRegistration = GlobalSettings.RegisterFilter(filter);
+            var filter = new FileExportFilter(FilterName, attributes);
+            var registration = GlobalSettings.RegisterFilter(filter);
 
             string filePath = Path.Combine(Directory.GetParent(repoPath).Parent.FullName, Guid.NewGuid().ToString() + ".blob");
             FileInfo contentFile = new FileInfo(filePath);
@@ -233,7 +231,31 @@ public void CanFilterLargeFiles()
 
             contentFile.Delete();
 
-            GlobalSettings.DeregisterFilter(filterRegistration);
+            GlobalSettings.DeregisterFilter(registration);
+        }
+
+        [Fact]
+        public void DoubleRegistrationFailsButDoubleDeregistrationDoesNot()
+        {
+            Assert.Equal(0, GlobalSettings.GetRegisteredFilters().Count());
+
+            var filter = new EmptyFilter(FilterName, attributes);
+            var registration = GlobalSettings.RegisterFilter(filter);
+
+            Assert.Throws<EntryExistsException>(() => { GlobalSettings.RegisterFilter(filter); });
+            Assert.Equal(1, GlobalSettings.GetRegisteredFilters().Count());
+
+            Assert.True(registration.IsValid, "FilterRegistration.IsValid should be true.");
+
+            GlobalSettings.DeregisterFilter(registration);
+            Assert.Equal(0, GlobalSettings.GetRegisteredFilters().Count());
+
+            Assert.False(registration.IsValid, "FilterRegistration.IsValid should be false.");
+
+            GlobalSettings.DeregisterFilter(registration);
+            Assert.Equal(0, GlobalSettings.GetRegisteredFilters().Count());
+
+            Assert.False(registration.IsValid, "FilterRegistration.IsValid should be false.");
         }
 
         private unsafe bool CharArrayAreEqual(char[] array1, char[] array2, int count)

LibGit2Sharp/Core/EncodingMarshaler.cs

@@ -59,7 +59,7 @@ public virtual Object MarshalNativeToManaged(IntPtr pNativeData)
 
         public static unsafe IntPtr FromManaged(Encoding encoding, String value)
         {
-            if (value == null)
+            if (encoding == null || value == null)
             {
                 return IntPtr.Zero;
             }

LibGit2Sharp/Core/Ensure.cs

@@ -133,7 +133,13 @@ private static readonly Dictionary<GitErrorCode, Func<string, GitErrorCode, GitE
         private static void HandleError(int result)
         {
             string errorMessage;
-            GitError error = NativeMethods.giterr_last().MarshalAsGitError();
+            GitError error = null;
+            var errHandle = NativeMethods.giterr_last();
+
+            if (errHandle != null && !errHandle.IsInvalid)
+            {
+                error = errHandle.MarshalAsGitError();
+            }
 
             if (error == null)
             {

LibGit2Sharp/Core/Proxy.cs

@@ -809,12 +809,12 @@ public static GitDiffDelta git_diff_get_delta(DiffSafeHandle diff, int idx)
 
         #region git_filter_
 
-        public static void git_filter_register(string name, FilterRegistration filterRegistration, int priority)
+        public static void git_filter_register(string name, IntPtr filterPtr, int priority)
         {
-            int res = NativeMethods.git_filter_register(name, filterRegistration.FilterPointer, priority);
+            int res = NativeMethods.git_filter_register(name, filterPtr, priority);
             if (res == (int)GitErrorCode.Exists)
             {
-                var message = string.Format("A filter with the name '{0}' is already registered", name);
+                var message = String.Format("A filter with the name '{0}' is already registered", name);
                 throw new EntryExistsException(message);
             }
             Ensure.ZeroResult(res);

LibGit2Sharp/Filter.cs

@@ -15,15 +15,10 @@ namespace LibGit2Sharp
     public abstract class Filter : IEquatable<Filter>
     {
         private static readonly LambdaEqualityHelper<Filter> equalityHelper =
-        new LambdaEqualityHelper<Filter>(x => x.Name, x => x.Attributes);
+            new LambdaEqualityHelper<Filter>(x => x.Name, x => x.Attributes);
         // 64K is optimal buffer size per https://technet.microsoft.com/en-us/library/cc938632.aspx
         private const int BufferSize = 64 * 1024;
 
-        private readonly string name;
-        private readonly IEnumerable<FilterAttributeEntry> attributes;
-
-        private readonly GitFilter gitFilter;
-
         /// <summary>
         /// Initializes a new instance of the <see cref="Filter"/> class.
         /// And allocates the filter natively.
@@ -46,6 +41,18 @@ protected Filter(string name, IEnumerable<FilterAttributeEntry> attributes)
                 stream = StreamCreateCallback,
             };
         }
+        /// <summary>
+        /// Finalizer called by the <see cref="GC"/>, deregisters and frees native memory associated with the registered filter in libgit2.
+        /// </summary>
+        ~Filter()
+        {
+            GlobalSettings.DeregisterFilter(this);
+        }
+
+        private readonly string name;
+        private readonly IEnumerable<FilterAttributeEntry> attributes;
+        private readonly GitFilter gitFilter;
+        private readonly object @lock = new object();
 
         private GitWriteStream thisStream;
         private GitWriteStream nextStream;

LibGit2Sharp/FilterRegistration.cs

@@ -9,26 +9,78 @@ namespace LibGit2Sharp
     /// </summary>
     public sealed class FilterRegistration
     {
-        internal FilterRegistration(Filter filter)
+        /// <summary>
+        /// Maximum priority value a filter can have. A value of 200 will be run last on checkout and first on checkin.
+        /// </summary>
+        public const int FilterPriorityMax = 200;
+        /// <summary>
+        /// Minimum priority value a filter can have. A value of 0 will be run first on checkout and last on checkin.
+        /// </summary>
+        public const int FilterPriorityMin = 0;
+
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="filter"></param>
+        /// <param name="priority"></param>
+        internal FilterRegistration(Filter filter, int priority)
         {
-            Ensure.ArgumentNotNull(filter, "filter");
-            Name = filter.Name;
+            System.Diagnostics.Debug.Assert(filter != null);
+            System.Diagnostics.Debug.Assert(priority >= FilterPriorityMin && priority <= FilterPriorityMax);
+
+            Filter = filter;
+            Priority = priority;
 
+            // marshal the git_filter strucutre into native memory
             FilterPointer = Marshal.AllocHGlobal(Marshal.SizeOf(filter.GitFilter));
             Marshal.StructureToPtr(filter.GitFilter, FilterPointer, false);
+
+            // register the filter with the native libary
+            Proxy.git_filter_register(filter.Name, FilterPointer, priority);
+        }
+        /// <summary>
+        /// Finalizer called by the <see cref="GC"/>, deregisters and frees native memory associated with the registered filter in libgit2.
+        /// </summary>
+        ~FilterRegistration()
+        {
+            // deregister the filter
+            GlobalSettings.DeregisterFilter(this);
+            // clean up native allocations
+            Free();
         }
 
+        /// <summary>
+        /// Gets if the registration and underlying filter are valid.
+        /// </summary>
+        public bool IsValid { get { return !freed; } }
+        /// <summary>
+        /// The registerd filters
+        /// </summary>
+        public readonly Filter Filter;
         /// <summary>
         /// The name of the filter in the libgit2 registry
         /// </summary>
-        public string Name { get; private set; }
+        public string Name { get { return Filter.Name; } }
+        /// <summary>
+        /// The priority of the registered filter
+        /// </summary>
+        public readonly int Priority;
+
+        private readonly IntPtr FilterPointer;
 
-        internal IntPtr FilterPointer { get; private set; }
+        private bool freed;
 
         internal void Free()
         {
-            Marshal.FreeHGlobal(FilterPointer);
-            FilterPointer = IntPtr.Zero;
+            if (!freed)
+            {
+                // unregister the filter with the native libary
+                Proxy.git_filter_unregister(Filter.Name);
+                // release native memory
+                Marshal.FreeHGlobal(FilterPointer);
+                // remember to not do this twice
+                freed = true;
+            }
         }
     }
 }