From 133505305a4b5d7938e70f125117a872739e474e Mon Sep 17 00:00:00 2001 From: Marius Ungureanu Date: Sat, 6 Dec 2014 13:49:10 +0100 Subject: [PATCH 1/4] Improve tracking toolset of improperly released unmanaged handles --- .travis.yml | 2 +- CI/build.msbuild | 15 +-- LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj | 3 +- LibGit2Sharp.Tests/TestHelpers/BaseFixture.cs | 23 +++- LibGit2Sharp/Core/Handles/SafeHandleBase.cs | 103 ++++++++++++++++-- LibGit2Sharp/ExtraDefine.targets | 7 ++ LibGit2Sharp/LibGit2Sharp.csproj | 3 +- build.libgit2sharp.cmd | 26 ++++- build.libgit2sharp.sh | 3 +- build.libgit2sharp.x64.cmd | 12 -- 10 files changed, 157 insertions(+), 40 deletions(-) create mode 100644 LibGit2Sharp/ExtraDefine.targets delete mode 100644 build.libgit2sharp.x64.cmd diff --git a/.travis.yml b/.travis.yml index 49106a6cd..e20403f5b 100644 --- a/.travis.yml +++ b/.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: diff --git a/CI/build.msbuild b/CI/build.msbuild index 5a94395e7..ea4d98b1d 100644 --- a/CI/build.msbuild +++ b/CI/build.msbuild @@ -9,14 +9,14 @@ - + - + - + @@ -29,10 +29,11 @@ + + Properties="Configuration=$(Configuration);TrackFileAccess=false;ExtraDefine=$(ExtraDefine)" /> diff --git a/LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj b/LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj index fc5285d5d..712b55a92 100644 --- a/LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj +++ b/LibGit2Sharp.Tests/LibGit2Sharp.Tests.csproj @@ -127,7 +127,8 @@ $(MSBuildProjectDirectory)\..\Lib\NativeBinaries - + + diff --git a/LibGit2Sharp.Tests/TestHelpers/BaseFixture.cs b/LibGit2Sharp.Tests/TestHelpers/BaseFixture.cs index dd2ab2c37..39c897a4e 100644 --- a/LibGit2Sharp.Tests/TestHelpers/BaseFixture.cs +++ b/LibGit2Sharp.Tests/TestHelpers/BaseFixture.cs @@ -16,6 +16,13 @@ public class BaseFixture : IPostTestDirectoryRemover, IDisposable { private readonly List directories = new List(); +#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 predicate, string message) diff --git a/LibGit2Sharp/Core/Handles/SafeHandleBase.cs b/LibGit2Sharp/Core/Handles/SafeHandleBase.cs index 81ff76838..b4ad98047 100644 --- a/LibGit2Sharp/Core/Handles/SafeHandleBase.cs +++ b/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 +{ + /// + /// Holds leaked handle type names reported by + /// + public static class LeaksContainer + { + private static readonly HashSet _typeNames = new HashSet(); + private static readonly object _lockpad = new object(); + + /// + /// Report a new leaked handle type name + /// + /// Short name of the leaked handle type. + public static void Add(string typeName) + { + lock (_lockpad) + { + _typeNames.Add(typeName); + } + } + + /// + /// Removes all previously reported leaks. + /// + public static void Clear() + { + lock (_lockpad) + { + _typeNames.Clear(); + } + } + + /// + /// Returns all reported leaked handle type names. + /// + public static IEnumerable 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 /// @@ -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)] diff --git a/LibGit2Sharp/ExtraDefine.targets b/LibGit2Sharp/ExtraDefine.targets new file mode 100644 index 000000000..b5ba508b5 --- /dev/null +++ b/LibGit2Sharp/ExtraDefine.targets @@ -0,0 +1,7 @@ + + + + + $(DefineConstants);$(ExtraDefine) + + diff --git a/LibGit2Sharp/LibGit2Sharp.csproj b/LibGit2Sharp/LibGit2Sharp.csproj index 8965442f6..347c8ea97 100644 --- a/LibGit2Sharp/LibGit2Sharp.csproj +++ b/LibGit2Sharp/LibGit2Sharp.csproj @@ -348,6 +348,7 @@ $(MSBuildProjectDirectory)\..\Lib\NativeBinaries + @@ -359,4 +360,4 @@ --> - \ No newline at end of file + diff --git a/build.libgit2sharp.cmd b/build.libgit2sharp.cmd index 54ecc1a26..8e85de196 100644 --- a/build.libgit2sharp.cmd +++ b/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% \ No newline at end of file +EXIT /B %ERRORLEVEL% diff --git a/build.libgit2sharp.sh b/build.libgit2sharp.sh index a64434fb2..84f1ba8a4 100755 --- a/build.libgit2sharp.sh +++ b/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 $? diff --git a/build.libgit2sharp.x64.cmd b/build.libgit2sharp.x64.cmd deleted file mode 100644 index 41702104b..000000000 --- a/build.libgit2sharp.x64.cmd +++ /dev/null @@ -1,12 +0,0 @@ -SETLOCAL - -SET BASEDIR=%~dp0 -SET FrameworkVersion=v4.0.30319 -SET FrameworkDir=%SystemRoot%\Microsoft.NET\Framework64 -SET CommitSha=%~1 - -"%FrameworkDir%\%FrameworkVersion%\msbuild.exe" "%BASEDIR%CI\build.msbuild" /property:CommitSha=%CommitSha% - -ENDLOCAL - -EXIT /B %ERRORLEVEL% \ No newline at end of file From 366edf6ac732408c088119ce0db61d567e8e4e1d Mon Sep 17 00:00:00 2001 From: nulltoken Date: Fri, 5 Dec 2014 22:57:40 +0100 Subject: [PATCH 2/4] Prevent leaking DiffSafeHandle on UnmatchedPathException --- LibGit2Sharp/Diff.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/LibGit2Sharp/Diff.cs b/LibGit2Sharp/Diff.cs index 4e0e398d6..70544f4a8 100644 --- a/LibGit2Sharp/Diff.cs +++ b/LibGit2Sharp/Diff.cs @@ -322,7 +322,15 @@ private DiffSafeHandle BuildDiffList(ObjectId oldTreeId, ObjectId newTreeId, Tre if (explicitPathsOptions != null) { - DispatchUnmatchedPaths(explicitPathsOptions, filePaths, matchedPaths); + try + { + DispatchUnmatchedPaths(explicitPathsOptions, filePaths, matchedPaths); + } + catch + { + diffList.Dispose(); + throw; + } } DetectRenames(diffList, compareOptions); From acb68a515ce14815a55489de19c7fb02b69ced0c Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sun, 7 Dec 2014 13:56:29 +0100 Subject: [PATCH 3/4] Include LibGit2Sharp.sln.DotSettings migration change --- LibGit2Sharp.sln.DotSettings | 1 + 1 file changed, 1 insertion(+) diff --git a/LibGit2Sharp.sln.DotSettings b/LibGit2Sharp.sln.DotSettings index 3d1a941ea..1b6065f39 100644 --- a/LibGit2Sharp.sln.DotSettings +++ b/LibGit2Sharp.sln.DotSettings @@ -10,4 +10,5 @@ False True True + True From 55b3c42c2fa3464941597db7ba913771a5158446 Mon Sep 17 00:00:00 2001 From: nulltoken Date: Sun, 7 Dec 2014 13:56:46 +0100 Subject: [PATCH 4/4] Minor cleanups --- LibGit2Sharp.Tests/LogFixture.cs | 5 +---- LibGit2Sharp/GlobalSettings.cs | 3 +-- LibGit2Sharp/Log.cs | 5 +---- 3 files changed, 3 insertions(+), 10 deletions(-) diff --git a/LibGit2Sharp.Tests/LogFixture.cs b/LibGit2Sharp.Tests/LogFixture.cs index 76654e3d3..b74fe97b0 100644 --- a/LibGit2Sharp.Tests/LogFixture.cs +++ b/LibGit2Sharp.Tests/LogFixture.cs @@ -1,7 +1,4 @@ -using System; -using LibGit2Sharp; -using LibGit2Sharp.Core; -using Xunit; +using Xunit; namespace LibGit2Sharp.Tests { diff --git a/LibGit2Sharp/GlobalSettings.cs b/LibGit2Sharp/GlobalSettings.cs index e63124fa7..60330b75c 100644 --- a/LibGit2Sharp/GlobalSettings.cs +++ b/LibGit2Sharp/GlobalSettings.cs @@ -1,6 +1,5 @@ using System; using LibGit2Sharp.Core; -using LibGit2Sharp.Handlers; namespace LibGit2Sharp { @@ -33,7 +32,7 @@ public static Version Version /// with the server This is not commonly /// used: some callers may want to re-use an existing connection to /// perform fetch / push operations to a remote. - /// + /// /// Note that this configuration is global to an entire process /// and does not honor application domains. /// diff --git a/LibGit2Sharp/Log.cs b/LibGit2Sharp/Log.cs index 23f79d93e..f8303fa01 100644 --- a/LibGit2Sharp/Log.cs +++ b/LibGit2Sharp/Log.cs @@ -1,7 +1,4 @@ -using System.Runtime.CompilerServices; -using LibGit2Sharp.Core; - -namespace LibGit2Sharp +namespace LibGit2Sharp { internal class Log {