Skip to content

Fix ResourceUpdater.Dispose() #120029

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 25, 2025
Merged

Fix ResourceUpdater.Dispose() #120029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1ce85d4
Always dispose PEReader in ResourceUpdater
jtschuster Sep 23, 2025
c94bec9
Practice good Dispose hygene
jtschuster Sep 23, 2025
f7aff95
Revert Dispose visibility changes
jtschuster Sep 23, 2025
5877756
Add unit test to validate dispose behavior
jtschuster Sep 24, 2025
22ab8ba
Make Dispose(bool) private
jtschuster Sep 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/installer/managed/Microsoft.NET.HostModel/ResourceUpdater.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace Microsoft.NET.HostModel
/// <summary>
/// Provides methods for modifying the embedded native resources in a PE image.
/// </summary>
public class ResourceUpdater : IDisposable
public sealed class ResourceUpdater : IDisposable
{
private readonly FileStream stream;
private readonly PEReader _reader;
Expand Down Expand Up @@ -331,12 +331,15 @@ public void Dispose()
GC.SuppressFinalize(this);
}

public void Dispose(bool disposing)
private void Dispose(bool disposing)
{
if (disposing && !leaveOpen)
if (disposing)
{
_reader.Dispose();
stream.Dispose();
if (!leaveOpen)
{
stream.Dispose();
}
}
}
}
Expand Down
21 changes: 20 additions & 1 deletion src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class TempFile : IDisposable
public TempFile()
{
_path = Path.GetTempFileName();
Stream = new FileStream(_path, FileMode.Open);
Stream = new FileStream(_path, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
}

public void Dispose()
Expand Down Expand Up @@ -169,6 +169,25 @@ void AddResource_AddTwoSameUShortTypeWithDifferName()
}
}

[Fact]
void DisposeWithLeaveOpenDisposesPEReader()
{
using var tempFile = GetCurrentAssemblyMemoryStream();
PEReader? peReader = null;

using (var updater = new ResourceUpdater(tempFile.Stream, leaveOpen: true))
{
FieldInfo? readerField = typeof(ResourceUpdater).GetField("_reader", BindingFlags.Instance | BindingFlags.NonPublic);
Assert.NotNull(readerField);
peReader = Assert.IsType<PEReader>(readerField.GetValue(updater));
_ = peReader.PEHeaders;
}

Assert.NotNull(peReader);

Assert.Throws<ObjectDisposedException>(() => _ = peReader!.PEHeaders);
}

[Fact]
void AddResourcesFromPEImage()
{
Expand Down
Loading