fix: accept BackupOrRestore and reject flushing through a closed handle - #1114
Merged
Merged
Conversation
…ndle - `OpenHandle` accepts `FileOptions` 0x02000000 (BackupOrRestore) on .NET 9 and later, as the runtime's options check does. .NET 8 still rejects it, and so does the mock there. - A stream on a handle throws `ObjectDisposedException` from `Flush`, `Flush(bool)` and `FlushAsync` once the handle is closed, as `FileStream` does. Disposing it still succeeds. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation is targeted, version-aware, and covered by real-versus-mock parity tests.
Review effort: Balanced
Findings: None
What changed in this PR
Aligns mock handle behavior with the real file system across supported .NET versions.
Changes:
- Accepts
BackupOrRestoreon .NET 9+. - Rejects flush operations after an owned handle closes.
- Adds parity tests for both behaviors.
| File | Description |
|---|---|
OpenHandleStreamTests.cs |
Tests flushing through a closed handle. |
OpenHandleTests.cs |
Tests version-specific BackupOrRestore handling. |
MockSafeFileHandleRegistry.cs |
Allows the option on .NET 9+. |
FileStreamMock.cs |
Treats a closed owned handle as disposed. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again. |
|
This is addressed in release v7.1.1. |
This was referenced Oct 1, 2026
This file contains hidden or 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
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.
Two small differences from the real file system in the handle support that shipped in 7.1.0.
BackupOrRestore:File.OpenHandlerejectedFileOptions0x02000000 (BackupOrRestore). The runtime's options check accepts it from .NET 9 onwards. .NET 8 still rejects it, and the mock now does the same per target.Flush,Flush(bool)andFlushAsyncafter the handle was closed.FileStreamthrowsObjectDisposedExceptionfor these, with the same "Cannot access a closed file." message. Disposing the stream still succeeds in both. The check sits inThrowIfDisposed, so the other members that use it follow suit.Both have tests in
Testably.Abstractions.Tests, so they also run against the real file system:OpenHandleTests: accepted on .NET 9+, rejected on .NET 8.OpenHandleStreamTests: flushing after the handle is closed.Testably.Abstractions.Testspasses in Release on net8.0 (12,810 tests), net9.0 (13,289) and net10.0 (13,319), real file system included, and so doesTestably.Abstractions.Testing.Tests.🤖 Generated with Claude Code
https://claude.ai/code/session_01TymoqrVYgZwdNSper5ZZB1