Skip to content

Scope FileType handles to Sessions (OPC 10000-5)#4048

Open
marcschier wants to merge 3 commits into
OPCFoundation:masterfrom
marcschier:marcschier/review-fix/session-file-handles
Open

Scope FileType handles to Sessions (OPC 10000-5)#4048
marcschier wants to merge 3 commits into
OPCFoundation:masterfrom
marcschier:marcschier/review-fix/session-file-handles

Conversation

@marcschier

Copy link
Copy Markdown
Collaborator

Description

This focused draft extracts the FileType handle ownership fix from #4021. It contains only the server FileSystem handle implementation, focused tests, and FileSystem client documentation.

Failure

FileHandle issued predictable per-file identifiers without recording the owning Session. A different Session that obtained a handle could read, write, seek, or close the stream, and open streams survived Session closure.

Fix

  • Generate non-zero opaque handles and bind every open stream to the Session that called Open.
  • Require a valid Session for FileType.Open, CreateFile when it requests an open handle, and all subsequent handle operations.
  • Reject a handle presented by another Session.
  • Close only the departing Session's handles from FileSystemNodeManager.SessionClosingAsync.
  • Document the Session ownership and lifetime.

OPC 10000-5 Section 6.2.5.2, Table 27 states that a fileHandle is valid only for the Session used to call Open and that the Server shall automatically close the file when that Session closes.

Tests

  • dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net10.0 --filter "FullyQualifiedName~FileHandleTests|FullyQualifiedName~FileObjectStateTests|FullyQualifiedName~DirectoryObjectStateTests" --no-restore: 71 passed.
  • dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj -f net48 --filter "FullyQualifiedName~FileHandleTests|FullyQualifiedName~FileObjectStateTests|FullyQualifiedName~DirectoryObjectStateTests" --no-restore: 71 passed.

Both builds emitted the same existing CA1873 warnings in unchanged Opc.Ua.Client files; the changed files produced no warnings.

Related Issues

Checklist

  • I have signed the CLA and read the CONTRIBUTING doc.
  • I have added tests that prove my fix is effective or that my feature works and increased code coverage.
  • I have added all necessary documentation.
  • I have verified that my changes do not introduce (new) build or analyzer warnings.
  • I ran all tests locally using the UA.slnx solution against at least .net framework and .net 10, and all passed.
  • I fixed all failing and flaky tests in the CI pipelines and all CodeQL warnings.
  • I have addressed all PR feedback received.

Bind opaque file handles to the opening Session, reject cross-Session access, and close owned handles when the Session closes.

Implements OPC 10000-5 Section 6.2.5.2 and documents the Session-scoped lifetime.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 10:48
Copilot AI review requested due to automatic review settings July 19, 2026 10:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Scopes server-side FileType file handles to the owning OPC UA Session, preventing cross-session handle reuse and ensuring handles are closed when their session ends (per OPC 10000-5).

Changes:

  • Bind open file streams to the Session that opened them and reject handle operations from other Sessions.
  • Add SessionClosingAsync cleanup to close only the departing Session’s file handles.
  • Update/extend unit tests and client documentation to cover/describe session-bound handle behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
tests/Opc.Ua.Server.Tests/FileSystem/FileObjectStateTests.cs Adds session-aware FileObjectState tests (opaque handles, session validation, session-closing behavior).
tests/Opc.Ua.Server.Tests/FileSystem/FileHandleTests.cs Updates FileHandle tests to pass session id and assert opaque/non-zero handles.
tests/Opc.Ua.Server.Tests/FileSystem/DirectoryObjectStateTests.cs Updates test context to include a Session for session-required operations.
src/Opc.Ua.Server/FileSystem/FileSystemNodeManager.cs Implements SessionClosingAsync to close handles owned by the closing session.
src/Opc.Ua.Server/FileSystem/FileObjectState.cs Requires Session for Open/Read/Write/Seek/Close; adds session-id extraction helper.
src/Opc.Ua.Server/FileSystem/FileHandle.cs Tracks session ownership per handle; generates opaque handles; adds CloseSession.
src/Opc.Ua.Server/FileSystem/DirectoryObjectState.cs Requires Session when CreateFile requests an open handle.
docs/FileSystemClient.md Documents that server file handles are opaque and session-bound with session-lifetime cleanup.

Comment thread src/Opc.Ua.Server/FileSystem/FileHandle.cs
Comment thread src/Opc.Ua.Server/FileSystem/FileSystemNodeManager.cs
Comment thread src/Opc.Ua.Server/FileSystem/FileHandle.cs
Comment thread src/Opc.Ua.Server/FileSystem/FileObjectState.cs Outdated
Comment thread src/Opc.Ua.Server/FileSystem/DirectoryObjectState.cs
@marcschier
marcschier marked this pull request as draft July 19, 2026 11:36
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 11:55
@marcschier
marcschier marked this pull request as draft July 19, 2026 12:12
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 14:12
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@marcschier
marcschier marked this pull request as draft July 19, 2026 16:47
@marcschier
marcschier marked this pull request as ready for review July 19, 2026 16:57
@marcschier

Copy link
Copy Markdown
Collaborator Author

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

@codecov

codecov Bot commented Jul 20, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.31507% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 73.59%. Comparing base (70c74e7) to head (672c518).
⚠️ Report is 21 commits behind head on master.

Files with missing lines Patch % Lines
src/Opc.Ua.Server/FileSystem/FileHandle.cs 98.59% 0 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4048      +/-   ##
==========================================
+ Coverage   73.55%   73.59%   +0.04%     
==========================================
  Files        1345     1345              
  Lines      180036   180182     +146     
  Branches    31677    31704      +27     
==========================================
+ Hits       132427   132612     +185     
+ Misses      36911    36858      -53     
- Partials    10698    10712      +14     
Files with missing lines Coverage Δ
...c/Opc.Ua.Server/FileSystem/DirectoryObjectState.cs 82.15% <100.00%> (+0.76%) ⬆️
src/Opc.Ua.Server/FileSystem/FileObjectState.cs 87.29% <100.00%> (+1.78%) ⬆️
.../Opc.Ua.Server/FileSystem/FileSystemNodeManager.cs 86.87% <100.00%> (+1.36%) ⬆️
src/Opc.Ua.Server/FileSystem/FileHandle.cs 96.57% <98.59%> (+0.27%) ⬆️

... and 35 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants