libgit2 / libgit2/libgit2sharp

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

Aberta
#2,193 2 comentários 0 reações 0 responsáveis Ver no GitHub

Ninguém assumiu esta issue ainda.

Linguagem predominante
C#
Estrelas
3.5k
Forks
925
Métricas de merge de PRs
Nenhum PR com merge em 30d

Descrição

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.

Guia de contribuição

Abrir o guia de contribuição

Primeiros passos

  1. Leia a issue inteira e depois o guia de contribuição do projeto.
  2. Comente na issue dizendo que vai assumir — evita que duas pessoas façam o mesmo trabalho.
  3. Faça um fork do repositório e trabalhe em uma branch.
  4. Abra um pull request que referencie o número da issue.

Direção de pesquisa

Rastreie NativeShutdownObject, Libgit2Object e NativeMethods.git_libgit2_init para entender a ordem do shutdown e da finalização de SafeHandle. Reproduza o cenário net472 server-GC com um Repository não descartado e observe a saída do processo. Está concluído quando o processo termina sem uma AccessViolation e o shutdown nativo e a limpeza dos handles não podem entrar em uma condição de corrida.

Escrita pelo modelo de indexação a partir do texto da issue.

Avaliação

Stack de tecnologia
csharp, git
Domínio
devtools
Tipo de issue
Bug
Dificuldade
4/5
Tempo estimado
3-5 dias
Status de atividade
Ativa
Clareza
Razoavelmente clara
Facilidade para iniciantes
52/100

Receba novas issues na sua caixa de entrada

Um resumo curto de issues do GitHub para quem está começando.