Skip to content

Commit 67e514f

Browse files
mpdreamzclaude
andcommitted
fix: address review of the handle registry and adopted streams
- `Flush`, `Flush(bool)` and `FlushAsync` on a stream whose handle has been closed throw `ObjectDisposedException`, as `FileStream` does; disposing it still succeeds. - The registry is published with `LazyInitializer`, so threads that open their first handle at the same time share one registry. - `OpenHandle` accepts `BackupOrRestore` (0x02000000), which the runtime's options check allows. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1
1 parent d257978 commit 67e514f

5 files changed

Lines changed: 74 additions & 11 deletions

File tree

‎src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileStream.cs‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -384,16 +384,21 @@ public override void WriteByte(byte value)
384384
/// <inheritdoc />
385385
public override void Flush()
386386
{
387+
ThrowIfHandleClosed();
387388
InternalFlush();
388389
}
389390

390391
/// <inheritdoc />
391392
public override void Flush(bool flushToDisk)
392-
=> InternalFlush();
393+
{
394+
ThrowIfHandleClosed();
395+
InternalFlush();
396+
}
393397

394398
/// <inheritdoc />
395399
public override Task FlushAsync(CancellationToken cancellationToken)
396400
{
401+
ThrowIfHandleClosed();
397402
InternalFlush();
398403
return Task.CompletedTask;
399404
}
@@ -477,20 +482,23 @@ private static bool ContentsEqual(byte[] left, byte[] right)
477482
/// </summary>
478483
private void ThrowIfHandleDenies(FileAccess required)
479484
{
480-
if (adoptedHandle == null)
485+
ThrowIfHandleClosed();
486+
if (adoptedHandle != null && !handleAccess.HasFlag(required))
481487
{
482-
return;
488+
throw CommonExceptions.AccessDenied(path);
483489
}
490+
}
484491

485-
if (adoptedHandle.IsClosed)
492+
/// <summary>
493+
/// Disposing the stream still succeeds once its handle is closed, so this is only checked by the operations the
494+
/// caller invokes.
495+
/// </summary>
496+
private void ThrowIfHandleClosed()
497+
{
498+
if (adoptedHandle is { IsClosed: true })
486499
{
487500
throw CommonExceptions.HandleIsClosed();
488501
}
489-
490-
if (!handleAccess.HasFlag(required))
491-
{
492-
throw CommonExceptions.AccessDenied(path);
493-
}
494502
}
495503

496504
private void RefreshBufferFromSharedFileData()

‎src/TestableIO.System.IO.Abstractions.TestingHelpers/MockFileSystem.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,8 @@ public MockFileSystem(IDictionary<string, MockFileData> files, MockFileSystemOpt
138138
/// <summary>
139139
/// The handles handed out by <see cref="MockFile.OpenHandle"/>. Not serialized, since they stand for open files.
140140
/// </summary>
141-
internal MockSafeFileHandles SafeFileHandles => safeFileHandles ??= new MockSafeFileHandles(this);
141+
internal MockSafeFileHandles SafeFileHandles
142+
=> Threading.LazyInitializer.EnsureInitialized(ref safeFileHandles, () => new MockSafeFileHandles(this));
142143
#endif
143144

144145
/// <summary>

‎src/TestableIO.System.IO.Abstractions.TestingHelpers/MockSafeFileHandles.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,8 @@ internal sealed class MockSafeFileHandles
3030
FileOptions.DeleteOnClose |
3131
FileOptions.SequentialScan |
3232
FileOptions.Encrypted |
33-
(FileOptions)0x20000000; // NoBuffering
33+
(FileOptions)0x20000000 | // NoBuffering
34+
(FileOptions)0x02000000; // BackupOrRestore
3435

3536
private readonly IMockFileDataAccessor mockFileDataAccessor;
3637
private readonly Dictionary<IntPtr, Entry> entries = new();

‎tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockFileOpenHandleTests.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,21 @@ await That(() => fileSystem.File.OpenHandle(FilePath, FileMode.Create, FileAcces
143143
.Throws<ArgumentOutOfRangeException>().WithParamName("preallocationSize");
144144
}
145145

146+
[TestCase((FileOptions)0x02000000)]
147+
[TestCase((FileOptions)0x20000000)]
148+
[TestCase(FileOptions.Asynchronous | FileOptions.WriteThrough)]
149+
public async Task OpenHandle_WithAnOptionTheRuntimeAccepts_ShouldOpen(FileOptions options)
150+
{
151+
var fileSystem = new MockFileSystem();
152+
fileSystem.AddFile(FilePath, new MockFileData("content"));
153+
154+
await That(() =>
155+
{
156+
using var handle = fileSystem.File.OpenHandle(FilePath, FileMode.Open, FileAccess.Read, FileShare.Read,
157+
options);
158+
}).DoesNotThrow();
159+
}
160+
146161
[Test]
147162
public async Task OpenHandle_PreallocationSizeOnExistingFile_ShouldThrowArgumentException()
148163
{

‎tests/TestableIO.System.IO.Abstractions.TestingHelpers.Tests/MockRandomAccessTests.cs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,5 +521,43 @@ public async Task Write_ConcurrentlyAtDistinctOffsets_ShouldKeepEveryWrite()
521521
await That(fileSystem.File.ReadAllBytes(FilePath)).IsEqualTo(expected)
522522
.Because("RandomAccess permits concurrent writes at distinct offsets");
523523
}
524+
[Test]
525+
public async Task FileStreamNew_FlushAfterTheHandleIsClosed_ShouldThrowObjectDisposedException()
526+
{
527+
var (fileSystem, handle) = Arrange(new byte[] { 1 });
528+
var stream = fileSystem.FileStream.New(handle, FileAccess.ReadWrite);
529+
handle.Dispose();
530+
531+
await That(() => stream.Flush()).Throws<ObjectDisposedException>();
532+
await That(() => stream.Flush(flushToDisk: true)).Throws<ObjectDisposedException>();
533+
await That(() => stream.FlushAsync()).Throws<ObjectDisposedException>();
534+
await That(() => stream.Dispose()).DoesNotThrow()
535+
.Because("disposing a stream whose handle is already closed succeeds, as it does for FileStream");
536+
}
537+
538+
[Test]
539+
public async Task OpenHandle_FromManyThreadsAtOnce_ShouldResolveEveryHandle()
540+
{
541+
for (var attempt = 0; attempt < 20; attempt++)
542+
{
543+
var fileSystem = new MockFileSystem();
544+
var paths = new string[16];
545+
for (var i = 0; i < paths.Length; i++)
546+
{
547+
paths[i] = XFS.Path($@"C:\temp\file{i}.bin");
548+
fileSystem.AddFile(paths[i], new MockFileData(new byte[] { (byte)i }));
549+
}
550+
551+
var handles = new SafeFileHandle[paths.Length];
552+
Parallel.For(0, paths.Length, i => handles[i] = fileSystem.File.OpenHandle(paths[i]));
553+
554+
for (var i = 0; i < handles.Length; i++)
555+
{
556+
using var open = handles[i];
557+
await That(fileSystem.RandomAccess.GetLength(open)).IsEqualTo(1)
558+
.Because("every handle is registered with the one registry of its file system");
559+
}
560+
}
561+
}
524562
}
525563
#endif

0 commit comments

Comments
 (0)