Skip to content

Commit 02509e4

Browse files
authored
Fix ResourceUpdater.Dispose() (#120029)
The apphost ResourceUpdater doesn't dispose of the PEReader when leaveOpen is true, but the leaveOpen should only apply to the FileStream passed to the constructor. The PEReader is not accessible from the creator and cannot be disposed of elsewhere. This causes some race conditions, especially in cases where the PEReader ends up mapping the file into memory. Fixes dotnet/sdk#50784
1 parent bb9df87 commit 02509e4

File tree

2 files changed

+27
-5
lines changed

2 files changed

+27
-5
lines changed

src/installer/managed/Microsoft.NET.HostModel/ResourceUpdater.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ namespace Microsoft.NET.HostModel
1313
/// <summary>
1414
/// Provides methods for modifying the embedded native resources in a PE image.
1515
/// </summary>
16-
public class ResourceUpdater : IDisposable
16+
public sealed class ResourceUpdater : IDisposable
1717
{
1818
private readonly FileStream stream;
1919
private readonly PEReader _reader;
@@ -331,12 +331,15 @@ public void Dispose()
331331
GC.SuppressFinalize(this);
332332
}
333333

334-
public void Dispose(bool disposing)
334+
private void Dispose(bool disposing)
335335
{
336-
if (disposing && !leaveOpen)
336+
if (disposing)
337337
{
338338
_reader.Dispose();
339-
stream.Dispose();
339+
if (!leaveOpen)
340+
{
341+
stream.Dispose();
342+
}
340343
}
341344
}
342345
}

src/installer/tests/Microsoft.NET.HostModel.Tests/ResourceUpdaterTests.cs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ class TempFile : IDisposable
2323
public TempFile()
2424
{
2525
_path = Path.GetTempFileName();
26-
Stream = new FileStream(_path, FileMode.Open);
26+
Stream = new FileStream(_path, FileMode.Open, FileAccess.ReadWrite, FileShare.None);
2727
}
2828

2929
public void Dispose()
@@ -169,6 +169,25 @@ void AddResource_AddTwoSameUShortTypeWithDifferName()
169169
}
170170
}
171171

172+
[Fact]
173+
void DisposeWithLeaveOpenDisposesPEReader()
174+
{
175+
using var tempFile = GetCurrentAssemblyMemoryStream();
176+
PEReader? peReader = null;
177+
178+
using (var updater = new ResourceUpdater(tempFile.Stream, leaveOpen: true))
179+
{
180+
FieldInfo? readerField = typeof(ResourceUpdater).GetField("_reader", BindingFlags.Instance | BindingFlags.NonPublic);
181+
Assert.NotNull(readerField);
182+
peReader = Assert.IsType<PEReader>(readerField.GetValue(updater));
183+
_ = peReader.PEHeaders;
184+
}
185+
186+
Assert.NotNull(peReader);
187+
188+
Assert.Throws<ObjectDisposedException>(() => _ = peReader!.PEHeaders);
189+
}
190+
172191
[Fact]
173192
void AddResourcesFromPEImage()
174193
{

0 commit comments

Comments
 (0)