libgit2 / libgit2/libgit2sharp

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

Abierto
#2,193 2 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Lenguaje dominante
C#
Estrellas
3.5k
Forks
925
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

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.

Guía de contribución

Abrir la guía de contribución

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Línea de trabajo

Rastrea NativeShutdownObject, Libgit2Object y NativeMethods.git_libgit2_init para comprender el orden del shutdown y la finalización de SafeHandle. Reproduce el escenario net472 server-GC con un Repository sin liberar y observa la salida del proceso. Se considera terminado cuando el proceso finaliza sin una AccessViolation y el shutdown nativo y la limpieza de handles no pueden producir una condición de carrera.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
csharp, git
Área
devtools
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Bastante claro
Aptitud para principiantes
52/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.