libgit2 / libgit2/libgit2sharp

Potential AccessViolation at process exit on .NET Framework - SafeHandle finalizers now race `git_libgit2_shutdown`

Open
#2,193 2 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
3.5k
Forks
925
PR merge metrics
No merged PRs in 30d

Description

Disclaimer: the below is AI-assisted but I'm reasonably confident it all checks out.

Starting with 0.31 (specifically #2127 - Use Safe Handles), a Repository that is still alive when a .NET Framework process exits crashes the process with an AccessViolation inside git_repository_free / git_odb_free:

ntdll!RtlpWaitOnCriticalSection+0xa6          <- DebugInfo == NULL: critical section already deleted
ntdll!RtlEnterCriticalSection+0x42
git2!<mwindow.c static>                        <- git_mwindow_put_pack / packfile free, global mwindow mutex
git2!<odb_pack.c static>                       <- pack backend free
git2!git_odb_free+0x7a
git2!git_repository__cleanup / git_repository_free
LibGit2Sharp.Core.Handles.RepositoryHandle.ReleaseHandle()   (or ObjectDatabaseHandle.ReleaseHandle)
System.Runtime.InteropServices.SafeHandle.Finalize()
clr!FinalizerThread::FinalizeAllObjects       <- main thread is in clr!EEShutDown

git_libgit2_shutdown has already run from NativeShutdownObject's finalizer and destroyed libgit2's global mutexes.

Root cause

NativeShutdownObject derives from CriticalFinalizerObject on purpose: commit 247ca1a3 (2012, "Make git_threads_shutdown run after git finalizers") fixed this very crash by relying on the CLR guarantee that all normal finalizers run before any critical finalizer, so shutdown was always last.

#2127 made Libgit2Object derive from SafeHandleZeroOrMinusOneIsInvalid. SafeHandle is itself a CriticalFinalizerObject, so every handle finalizer now runs in the same critical phase as the shutdown object, and the relative order is undefined (with server GC it depends on which heap each object was allocated on, so it is intermittent).

Repro (net472, server GC)

App.config with <gcServer enabled="true"/>; open a repository on ~24 threads, read a few commits on each, keep the Repository objects alive in a static, return from Main without disposing. Crashes 10/10 on 0.32.0. With one extra git_libgit2_init() call (so the shutdown finalizer never reaches count 0) it passes 10/10.

Suggested fix

Never tear the native library down from a finalizer at all. The finalizer-based shutdown only ever executes on .NET Framework (Core does not run finalizers at process exit), and there it is now actively harmful because SafeHandles are guaranteed to be finalized in the same phase. Options:

  1. Delete NativeShutdownObject; leave the process-lifetime git_libgit2_init reference outstanding (the OS reclaims everything at exit).
  2. Or keep it but make its finalizer a no-op when AppDomain.CurrentDomain.IsFinalizingForUnload() || Environment.HasShutdownStarted.

Workaround for consumers today: call the internal NativeMethods.git_libgit2_init once more via reflection so the refcount never drops to zero.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Trace NativeShutdownObject, Libgit2Object, and NativeMethods.git_libgit2_init to understand shutdown and SafeHandle finalization order. Reproduce the net472 server-GC scenario with an undisposed Repository and observe process exit. Done means the process exits without an AccessViolation while native shutdown and handle cleanup cannot race.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, git
Domain
devtools
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.