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

Abierto
  • ¿Algo no está bien?
#137,138 1 comentario 0 reacciones 0 asignados Ver en GitHub

Los mantenedores suelen responder en 1 día

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
4/5
Tiempo estimado
3-5 días
Aptitud para principiantes
50/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
csharp
Área
backend

Línea de trabajo

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.

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

Descripción

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.

Lenguaje dominante
C#
Estrellas
18.4k
Forks
5.8k
Merge medio
2 d 14 h
PR fusionados (30 d)
585

Preparar el entorno

Abrir en Codespaces

Inicia el contenedor de desarrollo del proyecto en tu navegador, con tu propia cuenta de GitHub.

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.

Más de dotnet/runtime

Todos los issues de dotnet/runtime

Issues similares

Más issues de C#

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.