search

jbevain / cecil

Cecil is a library to inspect, modify and create .NET programs and libraries.
MIT License
2.77k stars 630 forks source link

Fixed: SymWriter COM object is not released on exception #955

Closed OskiKervinen-MF closed 2 months ago

OskiKervinen-MF commented 4 months ago

The Problem

Accessing a PDB file edited with Cecil randomly produces errors about the file being locked. Problem observed when being used in coverlet: https://github.com/coverlet-coverage/coverlet/issues/1471

This is caused by an exclusive file handle held by the SymWriter COM object. It is not released before the COM object is released. In normal execution, NativePdbWriter.Write calls SymWriter.Close at the end, which in turn calls Marshal.ReleaseComObject, releasing the object and file handle. But if an exception causes NativePdbWriter.Write to never be called, there is no call to SymWriter.Close, which in turn means Marshal.ReleaseComObject is left uncalled.

The garbage collector will eventually destroy the object and thereby release the COM object, but that happens non-deterministically. The result was random file access issues with the PDB file.

The Solution

Luckily NativePdbWriter is IDisposable. I added a call to writer.Close to the Dispose. But now Close could be called twice, so I had to add a boolean to SymWriter to avoid releasing the resources twice.

OskiKervinen-MF commented 3 months ago

@jbevain Would it be better for me to report a separate Cecil issue about this, or is it enough that I link to the Coverlet issue this bug in Cecil causes?

The whole point of Dispose is to release unmanaged resources, so I would think this Pull Request makes sense without too much bookkeeping?

jbevain commented 2 months ago

Thank you for the PR!