Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Find out why / when LibGit2Sharp is crashing #1315

Closed
jcansdale opened this issue Nov 14, 2017 · 15 comments · Fixed by #1337
Closed

Find out why / when LibGit2Sharp is crashing #1315

jcansdale opened this issue Nov 14, 2017 · 15 comments · Fixed by #1337
Assignees

Comments

@jcansdale
Copy link
Collaborator

jcansdale commented Nov 14, 2017

This investigation is related to the following issue

GitHub extension crashes when viewing pull requests: #1306

  • Crash appears in LibGit2Sharp version: 0.24, 0.25.0-preview-0033
  • Crash not yet seen in LibGit2Sharp version: 0.23.0, 0.23.1

How to reproduce

Compile and run the following code in Release configuration.

        static void Main(string[] args)
        {
            var path = @"C:\source\github.com\github\VisualStudio";
            for (int count = 0; count < 1000; count++)
            {
                var status = new Repository(path).RetrieveStatus();
                Console.WriteLine(count + ": " + status.IsDirty);
            }
        }

Eventually something like this will happen:

image

Here is another very common exception:

System.AccessViolationException was unhandled
  HResult=-2147467261
  Message=Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
  Source=LibGit2Sharp
  StackTrace:
       at LibGit2Sharp.Core.NativeMethods.git_status_list_new(git_status_list*& git_status_list, git_repository* repo, GitStatusOptions options)
       at LibGit2Sharp.Core.Proxy.git_status_list_new(RepositoryHandle repo, GitStatusOptions options) in C:\projects\libgit2sharp\LibGit2Sharp\Core\Proxy.cs:line 2931
       at LibGit2Sharp.RepositoryStatus..ctor(Repository repo, StatusOptions options) in C:\projects\libgit2sharp\LibGit2Sharp\RepositoryStatus.cs:line 60
       at LibGit2Sharp.RepositoryExtensions.RetrieveStatus(IRepository repository) in C:\projects\libgit2sharp\LibGit2Sharp\RepositoryExtensions.cs:line 696
       at Libgit2Repro.Program.Main(String[] args) in c:\users\passp\onedrive\documents\visual studio 2015\Projects\Libgit2Repro\Libgit2Repro\Program.cs:line 20
       at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
       at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
       at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
       at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
       at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
       at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
       at System.Threading.ThreadHelper.ThreadStart()

This only seems to happen when compiled in Release configuration. When compiled in Debug configuration, it can run all 1000 iterations and never crash.

@jcansdale jcansdale self-assigned this Nov 14, 2017
@jcansdale
Copy link
Collaborator Author

jcansdale commented Nov 14, 2017

We upgraded from LibGit2Sharp 0.22 to 0.24 in PR #1248. The upgrade was in order to fix #1244 (the Failed to mmap issue). This fix was also confirmed with LibGit2Sharp 0.23.1. I upgraded to 0.24 because it was the latest stable version at the time.

Unfortunately after upgrading to v0.24, the crash #1306 started happening. This seemed to be fixed in PR #1311 by adding a using statement, but I'm pretty sure there's more to it than simply not disposing of the Repository object.

I'm now thinking that 0.23.1 would be a better choice because it fixes both issues. The merged PR #1311 might prevent a crash (or make one less likely) in one place, but the same crash might happen elsewhere.

@jcansdale
Copy link
Collaborator Author

@ethomson, @carlosmn, I was wondering if you might have seen the above crash before? There's a little more info here #1311.

@bwishan
Copy link

bwishan commented Nov 15, 2017

@jcansdale, I just upgraded 2.3.4.54 (from the private build ref'd in #1244) on top of VS 15.4.4 (also newly upgraded). I tried to load a PR and the plug-in crashed all of VS.

Here's the details from my event logs.

Application: devenv.exe
Framework Version: v4.0.30319
Description: The process was terminated due to an unhandled exception.
Exception Info: System.AccessViolationException
   at LibGit2Sharp.Core.NativeMethods.git_status_list_new(LibGit2Sharp.Core.git_status_list* ByRef, LibGit2Sharp.Core.git_repository*, LibGit2Sharp.Core.GitStatusOptions)
   at LibGit2Sharp.Core.Proxy.git_status_list_new(LibGit2Sharp.Core.Handles.RepositoryHandle, LibGit2Sharp.Core.GitStatusOptions)
   at LibGit2Sharp.RepositoryStatus..ctor(LibGit2Sharp.Repository, LibGit2Sharp.StatusOptions)
   at LibGit2Sharp.RepositoryExtensions.RetrieveStatus(LibGit2Sharp.IRepository)
   at GitHub.Services.PullRequestService.IsWorkingDirectoryClean(GitHub.Models.ILocalRepositoryModel)
   at GitHub.ViewModels.PullRequestDetailViewModel+<Load>d__100.MoveNext()
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore+MoveNextRunner.InvokeMoveNext(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore+MoveNextRunner.Run()
   at System.Threading.Tasks.SynchronizationContextAwaitTaskContinuation+<>c.<.cctor>b__8_0(System.Object)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at System.Windows.Threading.DispatcherOperation.InvokeInSecurityContext(System.Object)
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(System.Object)
   at System.Threading.ExecutionContext.RunInternal(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object, Boolean)
   at System.Threading.ExecutionContext.Run(System.Threading.ExecutionContext, System.Threading.ContextCallback, System.Object)
   at MS.Internal.CulturePreservingExecutionContext.Run(MS.Internal.CulturePreservingExecutionContext, System.Threading.ContextCallback, System.Object)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndWrapper.WndProc(IntPtr, Int32, IntPtr, IntPtr, Boolean ByRef)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(System.Object)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(System.Delegate, System.Object, Int32)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(System.Object, System.Delegate, System.Object, Int32, System.Delegate)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(System.Windows.Threading.DispatcherPriority, System.TimeSpan, System.Delegate, System.Object, Int32)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr, Int32, IntPtr, IntPtr)
Faulting application name: devenv.exe, version: 15.0.27004.2009, time stamp: 0x5a06819e
Faulting module name: git2-15e1193.DLL, version: 0.26.0.0, time stamp: 0x5947acc3
Exception code: 0xc0000005
Fault offset: 0x000471bf
Faulting process ID: 0x12450
Faulting application start time: 0x01d35e3daa85161a
Faulting application path: C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\devenv.exe
Faulting module path: C:\Program Files (x86)\Microsoft Visual Studio\2017\Enterprise\Common7\IDE\Extensions\GitHub\GitHub\lib\win32\x86\git2-15e1193.DLL
Report ID: 165a4136-846f-4e93-816d-e6c6c5b31e96
Faulting package full name: 
Faulting package-relative application ID: 

Also worth mentioning that this repro isn't consistent for me. VS closed and I was able to re-open the PR without issue.

@ethomson
Copy link

Ugh, that's terrible. No, I haven't seen this before.

We haven't made any changes to git_status in libgit2. What version of LibGit2Sharp are you upgrading from? My best guess is that this is an issue introduced during the .NET Standard migration, but I'm not certain...

@jcansdale
Copy link
Collaborator Author

jcansdale commented Nov 15, 2017

@ethomson,

What version of LibGit2Sharp are you upgrading from?

We moved from v0.22 to v0.24. This crash seems to happen in v0.24 and v0.25-preview, but not v0.23.1 and below. For the moment we're planning to revert to use v0.23.1 (#1316).

When executing inside of VS it crashes roughly 1/3 times on my machine. The above repro (#1315) can execute just a few iterations or 100s before it blows up. It seems very random.

@jcansdale
Copy link
Collaborator Author

jcansdale commented Nov 15, 2017

@bwishan thanks for the report and sorry about that!

LibGit2Sharp v0.23 seems to be the sweet spot. Could you try the .vsix linked here:
https://ci.appveyor.com/project/github-windows/visualstudio/build/2.3.5.312/artifacts

@bwishan
Copy link

bwishan commented Nov 15, 2017

No problem! Let me know if there's anything I can do to help.

@jcansdale
Copy link
Collaborator Author

I've also noticed this one happening randomly when running our tests (alas this is libgit2sharp v0.23.1).

Running GitHub.InlineReviews.UnitTests...

Unhandled Exception: System.AccessViolationException: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
   at LibGit2Sharp.Core.NativeMethods.git_revparse_ext(git_object*& obj, git_reference*& reference, git_repository* repo, String spec)
   at LibGit2Sharp.Core.Proxy.git_revparse_ext(RepositoryHandle repo, String objectish) in c:\Git\LibGit2Sharp-2\LibGit2Sharp\Core\Proxy.cs:line 2696
   at LibGit2Sharp.Core.Proxy.git_revparse_single(RepositoryHandle repo, String objectish) in c:\Git\LibGit2Sharp-2\LibGit2Sharp\Core\Proxy.cs:line 2717
   at LibGit2Sharp.Repository.Lookup(String objectish, GitObjectType type, LookUpOptions lookUpOptions) in c:\Git\LibGit2Sharp-2\LibGit2Sharp\Repository.cs:line 563
   at LibGit2Sharp.RepositoryExtensions.Lookup[T](IRepository repository, String objectish) in c:\Git\LibGit2Sharp-2\LibGit2Sharp\RepositoryExtensions.cs:line 32
   at GitHub.InlineReviews.UnitTests.TestDoubles.FakeDiffService.GetBlob(String path, String id) in C:\source\github.com\github\VisualStudio\test\GitHub.InlineReviews.UnitTests\TestDoubles\FakeDiffService.cs:line 97
   at GitHub.InlineReviews.UnitTests.TestDoubles.FakeDiffService.Diff(IRepository repo, String baseSha, String headSha, String path) in C:\source\github.com\github\VisualStudio\test\GitHub.InlineReviews.UnitTests\TestDoubles\FakeDiffService.cs:line 66
   at GitHub.InlineReviews.Services.PullRequestSessionService.<Diff>d__7.MoveNext() in C:\source\github.com\github\VisualStudio\src\GitHub.InlineReviews\Services\PullRequestSessionService.cs:line 57
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Runtime.CompilerServices.AsyncMethodBuilderCore.MoveNextRunner.Run()
   at Xunit.Sdk.MaxConcurrencySyncContext.RunOnSyncContext(SendOrPostCallback callback, Object state)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at Xunit.Sdk.ExecutionContextHelper.Run(Object context, Action`1 action)
   at Xunit.Sdk.MaxConcurrencySyncContext.WorkerThreadProc()
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ThreadHelper.ThreadStart(Object obj)

@grokys
Copy link
Contributor

grokys commented Nov 18, 2017

I've just seen these crashes on the latest version, which uses libgit2sharp 0.23.

@bording
Copy link

bording commented Nov 19, 2017

@ethomson Since the crash is happening with 0.24.0, that release was before the .NET Standard changes were merged, so there must be something else going on.

@jcansdale
Copy link
Collaborator Author

I've just seen these crashes on the latest version, which uses libgit2sharp 0.23.

The crash when running GitHub.InlineReviews.UnitTests or when opening a PR? 😨

@jcansdale
Copy link
Collaborator Author

@bording It seems that 0.23.1 also crashes in the same way, it just isn't easily triggered using the repro at the top (even after 1000s of iterations). It does however crash when running inside Visual Studio.

The strange thing is, this #1311 workaround does appear to fix it. I suspect the reason it works is that adding a using rearranges memory, rather being anything to do with the Dispose being called. Compiling the project in Debug mode has the same effect. 😕

@ethomson
Copy link

Right. I'm also a bit concerned about that using, I'll need to read through, but I'm worried that you're releasing the Repository multiple times if you do that. I don't remember the entire Dispose path offhand, but let's be sure to revisit it after we've tracked this down.

@jcansdale
Copy link
Collaborator Author

@ethomson,

Right. I'm also a bit concerned about that using, I'll need to read through, but I'm worried that you're releasing the Repository multiple times if you do that.

Here is a simplified version of what it's doing:

var repoPath = Repository.Discover(path);
using (var repo = new Repository(repoPath))
{
    var status = repo.RetrieveStatus();
    // do stuff with status
}

Were we leaking native resources when Dispose wasn't being called? Even if we were, I think this is a separate issue.

@davidchaine
Copy link

Bug is still present in version 26.0.0

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants