System.ComponentModel.Composition: CompositionLock.LockStateForRead/LockStateForWrite enter the lock before boxing its holder, so an OutOfMemoryException leaves the lock held

Aperta
  • Qualcosa non va?
#137,138 1 commento 0 reazioni 0 assegnatari Vedi su GitHub

I maintainer di solito rispondono entro 1 giorno

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
50/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
csharp
Ambito
backend

Direzione di ricerca

Start with CompositionLock.cs and the ReadLock and WriteLock constructors in Lock.Reader.cs and Lock.Writer.cs; trace how LockStateForRead and LockStateForWrite acquire and return their holders. Check existing tests for System.ComponentModel.Composition lock behavior, then verify that a failed holder allocation cannot leave the state lock held and that provider disposal still releases shared parts. The issue suggests allocating a reference holder before entering the lock or avoiding boxing.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

area-System.Threading untriaged
Description

CompositionLock.LockStateForRead() and CompositionLock.LockStateForWrite() return IDisposable. Each constructs the ReadLock / WriteLock struct, whose constructor enters the underlying ReaderWriterLockSlim (created with LockRecursionPolicy.NoRecursion), and only then boxes the struct to return it (CompositionLock.cs, Lock.Reader.cs, Lock.Writer.cs). The IL of the shipped 10.0.11 assembly:

LockStateForRead:  ldarg.0 | ldfld _isThreadSafe | brfalse.s | ldarg.0 | ldfld _stateLock | newobj ReadLock::.ctor | box Microsoft.Internal.ReadLock | ret | ldsfld _EmptyLockHolder | ret
LockStateForWrite: ldarg.0 | ldfld _isThreadSafe | brfalse.s | ldarg.0 | ldfld _stateLock | newobj WriteLock::.ctor | box Microsoft.Internal.WriteLock | ret | ldsfld _EmptyLockHolder | ret

If the box allocation throws OutOfMemoryException, the lock has been entered, but the caller's using never receives a holder, so nothing releases it. The thread then holds the state lock permanently:

  • CatalogExportProvider.Dispose, ComposablePartExportProvider.Dispose and ImportEngine.Dispose take the state write lock first. On that thread they throw LockRecursionException; on any other thread they would wait indefinitely. Either way the provider never disposes its shared parts.
  • Later state reads on that thread throw (NoRecursion), and writers on other threads wait.

We hit this in a memory-constrained container. A composition ran out of memory, and the later Dispose of the export provider threw LockRecursionException: Write lock may not be acquired with read lock held. That exception replaced the OutOfMemoryException for the caller. The trace of the original out-of-memory error was not kept. The ordering above is the only path we found that leaves a state lock held after the exception unwinds: a plain exception thrown inside composition leaves nothing held.

Reproduction Steps

An OutOfMemoryException cannot be aimed at the box allocation, so the repro takes the read lock through the same method and drops the holder. That leaves the same state as an out-of-memory error on the box.

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net10.0</TargetFramework>
    <Nullable>enable</Nullable>
    <ImplicitUsings>enable</ImplicitUsings>
  </PropertyGroup>
  <ItemGroup>
    <PackageReference Include="System.ComponentModel.Composition" Version="10.0.11" />
  </ItemGroup>
</Project>

using System.ComponentModel.Composition;
using System.ComponentModel.Composition.Hosting;
using System.Reflection;

// A thread-safe catalog export provider with one shared, disposable part, activated once.
var provider = new CatalogExportProvider(new TypeCatalog(typeof(Part)), isThreadSafe: true);
var container = new CompositionContainer(provider);
provider.SourceProvider = container;
_ = container.GetExportedValue<Part>();

// What an OutOfMemoryException on LockStateForRead()'s box leaves behind: the read lock held by this
// thread, with no holder to release it.
var compositionLock = typeof(CatalogExportProvider)
    .GetField("_lock", BindingFlags.Instance | BindingFlags.NonPublic)!
    .GetValue(provider)!;
compositionLock.GetType().GetMethod("LockStateForRead")!.Invoke(compositionLock, null);

try
{
    provider.Dispose(); // takes the state write lock first
    Console.WriteLine("disposed");
}
catch (Exception e)
{
    Console.WriteLine($"{e.GetType().FullName}: {e.Message}");
}

Console.WriteLine($"Part disposed: {Part.Disposed}");

[Export]
[PartCreationPolicy(CreationPolicy.Shared)]
public sealed class Part : IDisposable
{
    public static bool Disposed;
    public void Dispose() => Disposed = true;
}
Expected behavior

An exception thrown after the lock is entered does not leave it held. Dispose completes and disposes the shared part.

Actual behavior
System.Threading.LockRecursionException: Write lock may not be acquired with read lock held. This pattern is prone to deadlocks. Please ensure that read locks are released before taking a write lock. If an upgrade is necessary, use an upgrade lock in place of the read lock.
Part disposed: False
Regression?

Not as far as we know; the pattern is long-standing.

Known Workarounds

Before disposing, release the current thread's hold on the provider's state lock by reflection (CatalogExportProvider._lock → CompositionLock._stateLock → ReadWriteLock._thisLock), and likewise for the import engine and the container's part export provider.

Configuration
  • .NET 10.0.12; System.ComponentModel.Composition 10.0.11 (10.0.11+e2f47b0110ed922f21a1522da67279133ce28f32).
  • Repro run on macOS (arm64); the original failure was on Linux containers.
Other information

A possible fix is to allocate the holder before entering the lock, for example by making the holders LockStateForRead/LockStateForWrite return reference types that enter the lock after allocation, as CompositionLockHolder already is. Another is to return the struct types themselves, so that callers' using statements do not box.

Lingua principale
C#
Stelle
18.4k
Fork
5.8k
Merge medio
2g 14h
PR unite (30g)
585

Preparare l'ambiente

Apri in Codespaces

Avvia il container di sviluppo del progetto nel browser, con il tuo account GitHub.

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di dotnet/runtime

Tutte le issue di dotnet/runtime

Issue simili

Altre issue su C#

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.