Improve tracking toolset of improperly released unmanaged handles

Author: Therzok

.travis.yml

@@ -18,7 +18,7 @@ install: ./CI/travis.${TRAVIS_OS_NAME}.install.deps.sh
 
 # Build libgit2, LibGit2Sharp and run the tests
 script:
- - ./build.libgit2sharp.sh
+ - ./build.libgit2sharp.sh 'LEAKS_IDENTIFYING'
 
 # Only watch the development branch
 branches:

CI/build.msbuild

@@ -9,14 +9,14 @@
   <UsingTask AssemblyFile="$(RootDir)\Lib\xUnit\xunit.runner.msbuild.dll"
                TaskName="Xunit.Runner.MSBuild.xunit" />
   <Target Name="Clean">
-	<Message Text="Commit SHA = $(CommitSha)" />
+    <Message Text="Commit SHA = $(CommitSha)" />
 
-	<WriteLinesToFile Condition="'$(CommitSha)' != ''"
-		File="$(RootDir)\LibGit2Sharp\libgit2sharp_hash.txt"
-		Lines="$(CommitSha)"
-		Overwrite="true" />
+    <WriteLinesToFile Condition="'$(CommitSha)' != ''"
+      File="$(RootDir)\LibGit2Sharp\libgit2sharp_hash.txt"
+      Lines="$(CommitSha)"
+      Overwrite="true" />
 
-	<!-- Workaround for xbuild -->
+    <!-- Workaround for xbuild -->
     <Exec Condition=" ('$(OS)' != 'Windows_NT') " Command=" rm -r -f $(DeployFolder) " />
     <Exec Condition=" ('$(OS)' != 'Windows_NT') " Command=" rm -r -f $(TestBuildDir) " />
 
@@ -29,10 +29,11 @@
   </Target>
 
   <Target Name="Build" DependsOnTargets="Init">
+    <Message Text="ExtraDefine = $(ExtraDefine)" />
     <MSBuild
       Projects="$(RootDir)\LibGit2Sharp.sln"
       Targets="Build"
-      Properties="Configuration=$(Configuration);TrackFileAccess=false" />
+      Properties="Configuration=$(Configuration);TrackFileAccess=false;ExtraDefine=$(ExtraDefine)" />
   </Target>
 
   <Target Name="Test" DependsOnTargets="Build">

LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj

@@ -127,7 +127,8 @@
   <PropertyGroup>
     <NativeBinariesDirectory>$(MSBuildProjectDirectory)\..\Lib\NativeBinaries</NativeBinariesDirectory>
   </PropertyGroup>
-  <Import Project="$(MSBuildProjectDirectory)\..\LibGit2Sharp\CopyWindowsNativeDependencies.targets" />
+  <Import Project="..\LibGit2Sharp\CopyWindowsNativeDependencies.targets" />
+  <Import Project="..\LibGit2Sharp\ExtraDefine.targets" />
   <PropertyGroup>
     <PreBuildEvent>
     </PreBuildEvent>

LibGit2Sharp.Tests/TestHelpers/BaseFixture.cs

@@ -16,6 +16,13 @@ public class BaseFixture : IPostTestDirectoryRemover, IDisposable
     {
         private readonly List<string> directories = new List<string>();
 
+#if LEAKS_IDENTIFYING
+        public BaseFixture()
+        {
+            LeaksContainer.Clear();
+        }
+#endif
+
         static BaseFixture()
         {
             // Do the set up in the static ctor so it only happens once
@@ -190,14 +197,22 @@ public void Register(string directoryPath)
 
         public virtual void Dispose()
         {
-#if LEAKS
-            GC.Collect();
-#endif
-
             foreach (string directory in directories)
             {
                 DirectoryHelper.DeleteDirectory(directory);
             }
+
+#if LEAKS_IDENTIFYING
+            GC.Collect();
+            GC.WaitForPendingFinalizers();
+
+            if (LeaksContainer.TypeNames.Any())
+            {
+                Assert.False(true, string.Format("Some handles of the following types haven't been properly released: {0}.{1}"
+                    + "In order to get some help fixing those leaks, uncomment the define LEAKS_TRACKING in SafeHandleBase.cs{1}"
+                    + "and run the tests locally.", string.Join(", ", LeaksContainer.TypeNames), Environment.NewLine));
+            }
+#endif
         }
 
         protected static void InconclusiveIf(Func<bool> predicate, string message)

LibGit2Sharp/Core/Handles/SafeHandleBase.cs

@@ -1,18 +1,85 @@
-//#define LEAKS   //Uncomment this or add a conditional symbol to activate this mode
+
+// This activates a lightweight mode which will help put under the light
+// incorrectly released handles by outputing a warning message in the console.
+//
+// This should be activated when tests are being run of the CI server.
+//
+// Uncomment the line below or add a conditional symbol to activate this mode
+
+//#define LEAKS_IDENTIFYING
+
+// This activates a more throrough mode which will show the stack trace of the
+// allocation code path for each handle that has been improperly released.
+//
+// This should be manually activated when some warnings have been raised as
+// a result of LEAKS_IDENTIFYING mode activation.
+//
+// Uncomment the line below or add a conditional symbol to activate this mode
+
+//#define LEAKS_TRACKING
 
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Globalization;
+using System.Linq;
 using System.Runtime.ConstrainedExecution;
 using System.Runtime.InteropServices;
 using System.Threading;
 
+#if LEAKS_IDENTIFYING
+namespace LibGit2Sharp.Core
+{
+    /// <summary>
+    /// Holds leaked handle type names reported by <see cref="Core.Handles.SafeHandleBase"/>
+    /// </summary>
+    public static class LeaksContainer
+    {
+        private static readonly HashSet<string> _typeNames = new HashSet<string>();
+        private static readonly object _lockpad = new object();
+
+        /// <summary>
+        /// Report a new leaked handle type name
+        /// </summary>
+        /// <param name="typeName">Short name of the leaked handle type.</param>
+        public static void Add(string typeName)
+        {
+            lock (_lockpad)
+            {
+                _typeNames.Add(typeName);
+            }
+        }
+
+        /// <summary>
+        /// Removes all previously reported leaks.
+        /// </summary>
+        public static void Clear()
+        {
+            lock (_lockpad)
+            {
+                _typeNames.Clear();
+            }
+        }
+
+        /// <summary>
+        /// Returns all reported leaked handle type names.
+        /// </summary>
+        public static IEnumerable<string> TypeNames
+        {
+            get { return _typeNames.ToArray(); }
+        }
+    }
+}
+#endif
+
 namespace LibGit2Sharp.Core.Handles
 {
     internal abstract class SafeHandleBase : SafeHandle
     {
-#if LEAKS
+
+#if LEAKS_TRACKING
         private readonly string trace;
+        private readonly Guid id;
 #endif
 
         /// <summary>
@@ -27,26 +94,40 @@ protected SafeHandleBase()
         {
             NativeMethods.AddHandle();
             registered = 1;
-#if LEAKS
+
+#if LEAKS_TRACKING
+            id = Guid.NewGuid();
+            Trace.WriteLine(string.Format(CultureInfo.InvariantCulture, "Allocating {0} handle ({1})", GetType().Name, id));
             trace = new StackTrace(2, true).ToString();
 #endif
         }
 
-#if DEBUG
         protected override void Dispose(bool disposing)
         {
-            if (!disposing && !IsInvalid)
+            bool leaked = !disposing && !IsInvalid;
+
+#if LEAKS_IDENTIFYING
+            if (leaked)
             {
-                Trace.WriteLine(string.Format(CultureInfo.InvariantCulture, "A {0} handle wrapper has not been properly disposed.", GetType().Name));
-#if LEAKS
-                Trace.WriteLine(trace);
-#endif
-                Trace.WriteLine("");
+                LeaksContainer.Add(GetType().Name);
             }
+#endif
 
             base.Dispose(disposing);
-        }
+
+#if LEAKS_TRACKING
+            if (!leaked)
+            {
+                Trace.WriteLine(string.Format(CultureInfo.InvariantCulture, "Disposing {0} handle ({1})", GetType().Name, id));
+            }
+            else
+            {
+                Trace.WriteLine(string.Format(CultureInfo.InvariantCulture, "Unexpected finalization of {0} handle ({1})", GetType().Name, id));
+                Trace.WriteLine(trace);
+                Trace.WriteLine("");
+            }
 #endif
+        }
 
         // Prevent the debugger from evaluating this property because it has side effects
         [DebuggerBrowsable(DebuggerBrowsableState.Never)]

LibGit2Sharp/ExtraDefine.targets

@@ -0,0 +1,7 @@
+<?xml version="1.0" encoding="utf-8"?>
+<Project xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
+  <!-- Add extra defines to DefineConstants. -->
+  <PropertyGroup>
+    <DefineConstants Condition=" '$(ExtraDefine)' != '' ">$(DefineConstants);$(ExtraDefine)</DefineConstants>
+  </PropertyGroup>
+</Project>

LibGit2Sharp/LibGit2Sharp.csproj

@@ -348,6 +348,7 @@
     <NativeBinariesDirectory>$(MSBuildProjectDirectory)\..\Lib\NativeBinaries</NativeBinariesDirectory>
   </PropertyGroup>
   <Import Project="CopyWindowsNativeDependencies.targets" />
+  <Import Project="ExtraDefine.targets" />
   <PropertyGroup>
     <PreBuildEvent>
     </PreBuildEvent>
@@ -359,4 +360,4 @@
   <Target Name="AfterBuild">
   </Target>
   -->
-</Project>
+</Project>

build.libgit2sharp.cmd

@@ -1,12 +1,34 @@
+@ECHO OFF
+
+REM Sample usages:
+REM
+REM  Building and running tests
+REM  - build.libgit2sharp.cmd
+REM
+REM  Building, running tests and embedding the libgit2sharp commit sha
+REM  - build.libgit2sharp.cmd "6a6eb81272876fd63555165beef44de2aaa78a14"
+REM
+REM  Building and identifying potential leaks while running tests
+REM  - build.libgit2sharp.cmd "unknown" "LEAKS_IDENTIFYING"
+
+
 SETLOCAL
 
 SET BASEDIR=%~dp0
 SET FrameworkVersion=v4.0.30319
 SET FrameworkDir=%SystemRoot%\Microsoft.NET\Framework
+
+if exist "%SystemRoot%\Microsoft.NET\Framework64" (
+  SET FrameworkDir=%SystemRoot%\Microsoft.NET\Framework64
+)
+
+ECHO ON
+
 SET CommitSha=%~1
+SET ExtraDefine=%~2
 
-"%FrameworkDir%\%FrameworkVersion%\msbuild.exe" "%BASEDIR%CI\build.msbuild" /property:CommitSha=%CommitSha%
+"%FrameworkDir%\%FrameworkVersion%\msbuild.exe" "%BASEDIR%CI\build.msbuild" /property:CommitSha=%CommitSha% /property:ExtraDefine="%ExtraDefine%"
 
 ENDLOCAL
 
-EXIT /B %ERRORLEVEL%
+EXIT /B %ERRORLEVEL%

build.libgit2sharp.sh

@@ -2,6 +2,7 @@
 
 LIBGIT2SHA=`cat ./LibGit2Sharp/libgit2_hash.txt`
 SHORTSHA=${LIBGIT2SHA:0:7}
+EXTRADEFINE="$1"
 
 rm -rf libgit2/build
 mkdir libgit2/build
@@ -26,6 +27,6 @@ export MONO_OPTIONS=--debug
 
 echo $DYLD_LIBRARY_PATH
 echo $LD_LIBRARY_PATH
-xbuild CI/build.msbuild /t:Deploy
+xbuild CI/build.msbuild /target:Deploy /property:ExtraDefine="$EXTRADEFINE"
 
 exit $?