Fixed: SymWriter COM object is not released on exception #955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Problem: Problem observed when being used in coverlet: coverlet-coverage/coverlet#1471 If an exception caused `NativePdbWriter.Write` to never be called, it would not call `SymWriter.Close`, which in turn meant `Marshal.ReleaseComObject` was left uncalled. The garbage collector will eventually destroy the object and thereby release the COM object, but that happens non-deterministically. The COM object is holding to a file handle, which prevents other operations on the written file until it is released. The result was random file access issues. The Solution: Luckily NativePdbWriter is IDisposable. I added a call to `writer.Close` there. But now it could be called twice, so I had to add a boolean to SymWriter to avoid releasing everything twice.
|
@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? |
|
Thank you for the PR! |
This was referenced Nov 2, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The Problem
Accessing a PDB file edited with Cecil randomly produces errors about the file being locked.
Problem observed when being used in coverlet:
coverlet-coverage/coverlet#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.WritecallsSymWriter.Closeat the end,which in turn calls
Marshal.ReleaseComObject, releasing the object and file handle.But if an exception causes
NativePdbWriter.Writeto never be called, there is no call toSymWriter.Close,which in turn means
Marshal.ReleaseComObjectis 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
NativePdbWriterisIDisposable. I added a call towriter.Closeto theDispose. But nowClosecould be called twice, so I had to add a boolean to SymWriter to avoid releasing the resources twice.