libgit2 / libgit2/libgit2sharp

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

Ouverte
#2,193 2 commentaires 0 réactions 0 personnes assignées Voir sur GitHub

Personne n'a encore pris cette issue.

Langage dominant
C#
Étoiles
3.5k
Forks
925
Métriques de merge des PR
Aucune PR mergée en 30 j

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.

Guide de contribution

Ouvrir le guide de contribution

Par où commencer

  1. Lisez l'issue en entier, puis le guide de contribution du projet.
  2. Signalez en commentaire que vous la prenez — cela évite que deux personnes fassent le même travail.
  3. Forkez le dépôt et travaillez sur une branche.
  4. Ouvrez une pull request qui référence le numéro de l'issue.

Piste de recherche

Suivez NativeShutdownObject, Libgit2Object et NativeMethods.git_libgit2_init afin de comprendre l’ordre du shutdown et de la finalisation de SafeHandle. Reproduisez le scénario net472 server-GC avec un Repository non libéré et observez l’arrêt du processus. C’est terminé lorsque le processus se termine sans AccessViolation et que le shutdown natif et le nettoyage des handles ne peuvent pas entrer en concurrence.

Rédigé par le modèle d'indexation à partir du texte de l'issue.

Évaluation

Stack technique
csharp, git
Domaine
devtools
Type d'issue
Bug
Difficulté
4/5
Temps estimé
3-5 jours
Activité
Active
Clarté
Plutôt claire
Accessibilité débutants
52/100

Recevez les nouvelles issues par e-mail

Un résumé court des issues GitHub adaptées aux débutants.