Skip to content

Merge fixes from master378 to Release/1.5.378 - #4457

Merged
mrsuciu merged 19 commits into
release/1.5.378from
master378
Sep 11, 2026
Merged

Merge fixes from master378 to Release/1.5.378#4457
mrsuciu merged 19 commits into
release/1.5.378from
master378

Conversation

@romanett

@romanett romanett commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Description

Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history.

Related Issues

Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here.

If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR)).

  • Fixes #github-issue-number, ...

Checklist

Put an x in the boxes that apply. You can complete these step by step after opening the PR.

  • 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.

marcschier and others added 17 commits July 21, 2026 12:54
# Description

Fixes a server-side lost wake-up in `SessionPublishQueue` that can
permanently stop monitored-item notifications while the Session and
SecureChannel remain healthy.

In 1.5.378, `PublishAsync` checked for a ready Subscription under
`m_subscriptionPublishLock` and then queued the Publish request under
`m_lock`. A Subscription could become ready between those operations, be
marked `ReadyToPublish` because no request was queued yet, and then
leave the newly queued request stranded. Later timer ticks skipped
assignment because the Subscription was already marked ready.

This change backports the relevant `SessionPublishQueue` correction from
#3611 and incorporates the review follow-up:

- Makes the ready-Subscription check and Publish-request enqueue atomic
under `m_lock`.
- Synchronizes `PublishCompleted`, `Requeue`, and timer-driven
assignment with the same lock.
- Removes `m_subscriptionPublishLock`.
- Invokes `SessionClosed` callbacks outside the queue lock.
- Retries timer-driven assignment for already-ready, non-publishing
Subscriptions instead of skipping them.
- Adds deterministic regression tests for the lost-wakeup and requeue
paths.

## Related Issues

- Fixes #3997
- Backports the relevant queue fix from #3611

## Testing

- The focused `SessionPublishQueueRaceTests` fixture passes on net472,
net48, net8.0, net9.0, and net10.0.
- Earlier branch validation ran the full `UA.slnx` on net10.0 and net48
with zero failures.
- Three pre-existing Quickstarts CA1823 warnings remain unchanged.
## Proposed changes

`ApplicationConfiguration.TraceConfiguration` still existed as a live
configuration surface after `ITelemetryContext` was introduced, which
made the migration path unclear. This change makes that legacy surface
explicitly discoverable as transitional API and points consumers to the
telemetry-based replacement.

- **Public API guidance**
- Marks `ApplicationConfiguration.TraceConfiguration` as obsolete with a
message directing consumers to `ITelemetryContext` and `ILogger`-based
diagnostics.
- Keeps the property functional so existing configuration loading and
legacy trace application continue to work during migration.

- **Internal compatibility**
- Adds targeted `CS0618` suppressions at the remaining internal legacy
call sites that intentionally preserve `TraceConfiguration` behavior.
- Avoids broad warning suppression; only the compatibility path is
annotated.

- **Coverage**
- Adds a focused reflection-based test that verifies the property is
marked obsolete and that the migration message stays stable.

```csharp
[Obsolete("Use ITelemetryContext and ILogger-based diagnostics instead.")]
public TraceConfiguration TraceConfiguration { get; set; }
```

## Related Issues

## Types of changes

What types of changes does your code introduce?
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR._

- [ ] Bugfix (non-breaking change which fixes an issue)
- [x] Enhancement (non-breaking change which adds functionality)
- [x] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist

_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR. If you're unsure about any of them, don't hesitate to
ask. We're here to help! This is simply a reminder of what we are going
to look for before merging your code._

- [ ] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines. 
- [x] I fixed all introduced issues with CodeQL and LGTM.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.

## Further comments

This is intentionally narrow in scope: it clarifies the migration path
without removing the legacy trace pipeline yet. The change is limited to
surfacing the deprecation at the public configuration boundary and
isolating the remaining intentional legacy usages.

<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #4056

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
## Proposed changes

The master378 publish timer reset timestamps for already-ready
subscriptions, breaking oldest-first ordering among equal-priority
subscriptions.

- Restore the ready-subscription guard.
- Recheck readiness under the queue lock to prevent concurrent timestamp
resets.
- Replace invalid requeue coverage with a deterministic ordering
regression test.

## Related Issues

## Types of changes

What types of changes does your code introduce?
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR._

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] Enhancement (non-breaking change which adds functionality)
- [x] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist

_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR. If you're unsure about any of them, don't hesitate to
ask. We're here to help! This is simply a reminder of what we are going
to look for before merging your code._

- [ ] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines. 
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.

## Further comments

Retains the single-lock lost-wakeup correction from the preceding change
while preserving subscription scheduling timestamps.

<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #3997

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: marcschier <marcschier@hotmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 892a02b2-efa4-4a3b-bc9a-5e42103f4c26
… Browser.BrowseAsync (#4168)

Re-submission of #4167 against `master378`.

# Description

#4167 was merged directly into `release/1.5.378`. Maintenance fixes for
the 1.5.378 lineage have to land on `master378` first, so the commit was
removed from `release/1.5.378` again and the fix is submitted here
instead.

Servers 1.6.0 and higher return a **zero length** continuation point in
the `Browse` response to indicate that no further references are
available. `Browser.BrowseAsync` only checked the continuation point for
`null`, so it issued a `BrowseNext` call with an empty continuation
point, which the server rejects with `BadContinuationPointInvalid`.

The fix follows the continuation point only if it actually contains
data. This is consistent with the already existing
`continuationPoint?.Length > 0` check in the
`OperationCanceledException` filter of the same method.

Credit for the original fix goes to @KarenKrill (#4167).

## Related Issues

- Fixes #3698
- Supersedes #4167

## Checklist

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] I ran the affected tests locally.
- [ ] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [ ] I have addressed **all** PR feedback received.

## Further comments

New unit tests in `Tests/Opc.Ua.Client.Tests/BrowserUnitTests.cs` pin
the behaviour for `null`, empty and non empty continuation points.
Verified that the new tests fail without the one line change and pass
with it:

`dotnet test Tests\Opc.Ua.Client.Tests\Opc.Ua.Client.Tests.csproj -f
net10.0 --filter "FullyQualifiedName~BrowserUnitTests"` -> `Failed: 0,
Passed: 3`

The issue does not exist on `master` (2.0), where the `ByteString`
refactoring already handles this.

Co-authored-by: KarenKrill <74286712+KarenKrill@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 75db5878-10f9-4099-993a-5eb969fa080e
## Proposed changes

This change adds a focused guide for users building their own NodeSet2
models with the .NET Standard stack. It documents the intended workflow
discussed in the issue: author in ModelDesign, generate artifacts with
ModelCompiler, and create runtime instances in `CreateAddressSpace`
instead of hand-maintaining generated NodeSet2 content.

- **New workflow guide**
  - Added `Docs/CustomNodeSet2Workflow.md`
  - Documents the recommended split between:
    - `ModelDesign` as the source of truth
- ModelCompiler outputs (`NodeSet2`, CSV, generated classes/constants,
predefined nodes)
    - runtime instantiation in server code

- **Runtime guidance clarified**
- Explains when to use `CreateAddressSpace` vs.
`AddBehaviourToPredefinedNode`
- Calls out the common case of selecting one machine variant at runtime
from several modeled types
- Describes why generated instance children receive distinct NodeIds and
why manually reusing explicit NodeIds causes confusion

- **Modeling guidance clarified**
- Recommends making inherited optional children mandatory in the derived
type when the server always exposes them
- Positions generated `NodeSet2` as an output artifact, not the primary
hand-edited source

Example from the documented approach:

```xml
<opc:ObjectType SymbolicName="GearMachineType" BaseType="MachineTool:MachineToolType">
  <opc:Children>
    <opc:Property SymbolicName="DI:Model"
                  DataType="ua:LocalizedText"
                  ModellingRule="Mandatory" />
  </opc:Children>
</opc:ObjectType>
```

## Related Issues

## Types of changes

What types of changes does your code introduce?
_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR._

- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] Enhancement (non-breaking change which adds functionality)
- [ ] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [x] Documentation Update (if none of the other choices apply)

## Checklist

_Put an `x` in the boxes that apply. You can also fill these out after
creating the PR. If you're unsure about any of them, don't hesitate to
ask. We're here to help! This is simply a reminder of what we are going
to look for before merging your code._

- [ ] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [ ] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines. 
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.

## Further comments

The guide is anchored to existing quickstart patterns already present in
the repository, so it explains the current intended workflow without
introducing new tooling or changing runtime behavior.

<!-- START COPILOT CODING AGENT SUFFIX -->

- Fixes #4169

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Fixed timer leaks in ChannelAsyncOperation.EndAsync

## Proposed changes

- Added linked cancellation token source for delay task (runtimes <=
.NET 6.0)
- Added delay task cancellation (runtimes <= .NET 6.0)

## Related Issues

- Fixes #3668

## Types of changes

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] Enhancement (non-breaking change which adds functionality)
- [ ] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist

- [x] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [ ] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines. 
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [ ] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [ ] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.

## Further comments

Fix already accepted in master branch, please add it to latest 1.5.378
version

---------

Co-authored-by: Suciu Mircea Adrian <Mircea-Adrian.Suciu@softing.com>
Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
## Proposed changes

Backport commit
a5dd6ea
(#4047) to `master378`.

OPC UA defines zero as no notification limit. The message builder
compared the queued count directly with zero, so an effective zero limit
emitted no event or data-change notifications and left the queues
pending. Separately, the revision logic treated a configured Server
maximum of zero as a numeric cap, revising a non-zero Client request to
zero and silently removing the Client's requested limit.

### Fix
- Treat an effective zero limit as unlimited while draining event and
data-change queues.
- When the Client requests zero, use the Server's configured maximum,
which may itself remain zero/unlimited.
- When the Server maximum is zero, preserve a non-zero Client request,
including `uint.MaxValue`.
- When the Server maximum is finite, cap zero, `uint.MaxValue`, and
larger finite Client requests to that maximum.
- Cover the same revision matrix through both CreateSubscription and
ModifySubscription.

## Related Issues

- Backport of #4047 (commit `a5dd6eae7cc1ab074d62d723194285952560555f`)

## Types of changes

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] Enhancement (non-breaking change which adds functionality)
- [x] Test enhancement (non-breaking change to increase test coverage)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected, requires version increase of
Nuget packages)
- [ ] Documentation Update (if none of the other choices apply)

## Checklist

- [x] I have read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf).
- [x] I ran tests locally with my changes, all passed.
- [ ] I fixed all failing tests in the CI pipelines. 
- [ ] I fixed all introduced issues with CodeQL and LGTM.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added necessary documentation (if appropriate).
- [ ] Any dependent changes have been merged and published in downstream
modules.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…) (#4270)

Backports the browse continuation point limit fix from master (commit
1eb5e0c, PR #4036) to `master378`.

## Problem
An off-by-one error in browse continuation-point retention allowed one
extra browse continuation point to remain active. The eviction check
only removed the oldest point when the count *exceeded*
`MaxBrowseContinuationPoints`, so a session could hold
`MaxBrowseContinuationPoints + 1` points.

## Fix
In `Session.SaveContinuationPoint`, evict the oldest point as soon as
the session *reaches* the configured limit (`>=` instead of `>`), so at
most `MaxBrowseContinuationPoints` points remain active.

## Tests
Adds
`SessionContinuationPointsTests.SaveContinuationPointEvictsOldestWhenCountReachesConfiguredLimit`,
a regression test that saves one more than the configured limit and
asserts the oldest point is evicted while exactly the configured number
are retained. Verified it fails against the pre-fix code and passes with
the fix.

> Note: The upstream commit modified the refactored
`SessionContinuationPoints` class, which does not exist on `master378`.
The equivalent change here is applied to
`Session.SaveContinuationPoint`, and the regression test is written
against the `master378` `Session`/`ISession` API.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…78] (#4269)

Backport of
8f40876
from `master` to `master378`.

### Summary
Makes `Session.ValidateServerEndpoints()` compare `ServerEndpoints` and
`UserIdentityTokens` as unordered sets and adds regression tests
verifying `OpenAsync()` no longer rejects legal servers that return the
same endpoint data in a different order.

### Problem
Per OPC UA Part 4, the client validates the endpoint set returned by
`CreateSessionResponse.ServerEndpoints` against the set observed during
discovery. The spec describes these as a *set* of endpoint descriptions
filtered by transport profile, not an ordered array. Previously the
validation compared `m_discoveryServerEndpoints[ii]` against
`serverEndpoints[ii]` by index, and `UserIdentityTokens[jj]` by index. A
server returning the same legal endpoints/token policies in a different
order could be wrongly rejected with `BadSecurityChecksFailed`.

### Changes
- Replace index-based `ServerEndpoints` validation with unordered
matching on the same endpoint fields already validated by the client.
- Compare `UserIdentityTokens` as an unordered multiset.
- Extend the client session test scaffolding so discovery endpoints can
be supplied explicitly.
- Add regression tests for reordered `ServerEndpoints` and reordered
`UserIdentityTokens`.

### Notes
Adapted to the `master378` API surface (`EndpointDescriptionCollection`
/ `UserTokenPolicyCollection` / `StringCollection` / `byte[]` server
nonce) and the `Libraries/`/`Tests/` layout. Client tests build and the
new tests pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…) [backport to master378] (#4268)

### Summary

Backport of commit 399747f (#4022) from `master` to `master378`.

Fixes `Browse` and `BrowseNext` so they do not return a usable
continuation point when the server reports `BadNoContinuationPoints`,
and adds regression tests covering both code paths.

### Problem

Per OPC UA Part 4, if the server cannot allocate another continuation
point for a `Browse`/`BrowseNext` operation, the result should be
`Bad_NoContinuationPoints` and the server must not also return a usable
continuation point. Previously `FetchReferences` could return
`BadNoContinuationPoints` while still passing a live continuation point
back to its callers, producing a contradictory response
(`BadNoContinuationPoints` with a non-empty `ContinuationPoint`), and
`BrowseNext` could overwrite the error with `Good`.

### Changes

- Dispose and clear the continuation point when `FetchReferences` hits
the no-continuation-points path.
- Only copy a continuation point into `Browse` and `BrowseNext` results
when the operation completed successfully
(`ServiceResult.IsGood(error)`).
- Add deterministic regression tests for both `Browse` and `BrowseNext`,
adapted to the `master378` server API.

### Notes

The upstream commit targets the v2.0 APIs (`ArrayOf`/`ByteString`); the
source fix and tests here were adapted to the `master378` API (`byte[]`
continuation points, `BrowseResultCollection`/`ByteStringCollection`,
4-arg `OperationContext` constructor). Both new tests pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
)

Backport of 70c74e7 from `master` to `master378`.

### Summary

Scopes `Cancel` request matching to the caller's session and adds a
regression test verifying a request handle can no longer cancel work
belonging to another session.

### Problem

Per OPC UA Part 4, `Cancel` applies to a request handle from the same
session that issued the original request. Previously
`StandardServer.CancelAsync` validated the caller's session but
forwarded only the raw `requestHandle` to
`RequestManager.CancelRequests`, which iterated the global
outstanding-request table and canceled every request whose
`ClientHandle` matched — regardless of owning session. Since request
handles are only session-scoped and different sessions can reuse the
same value, one session could cancel another session's in-flight
request.

### Changes

- Pass the validated caller session id from `StandardServer.CancelAsync`
into `RequestManager.CancelRequests`.
- Restrict `RequestManager.CancelRequests` to requests whose `SessionId`
**and** `ClientHandle` both match.
- Update the existing request-manager test call and add
`CancelRequestsDoesNotCancelMatchingHandleFromDifferentSession`
regression test.

### Notes on the backport

The `master378` branch predates the `RequestLifetime` cancellation model
and the `SessionPublishQueueTests` present on `master`, so those parts
of the original commit do not apply. The core fix and the regression
test were adapted to this branch's `SetStatusCode`/`OperationStatus`
model. `Opc.Ua.Server.Tests` builds clean and both `RequestManagerTests`
pass.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…#4266)

Ports the ContinuationPoint fix from master commit a568a7e (PR #4057)
back to master378.

## What changed
`MasterNodeManager.Browse`/`BrowseNext` now explicitly set
`BrowseResult.ContinuationPoint = default` (i.e. null) when no
continuation point is present, instead of leaving it as an empty array.
This aligns server behavior with the existing 1.5.xxx contract.

Three call sites in
`Libraries/Opc.Ua.Server/NodeManager/MasterNodeManager.cs` were updated:
- initial browse result
- bad validation result in BrowseNext
- good result in BrowseNext

## Tests
Added `Tests/Opc.Ua.Server.Tests/MasterNodeManagerBrowseTests.cs`,
porting the relevant test coverage from #4057 and adapting it to the
master378 server test API (`BrowseResultCollection`, `byte[]`
continuation point, `OperationContext` ctor):
- `BrowseAsyncCompletedBrowseReturnsNullContinuationPointAsync` — a
completed browse returns Good and a null continuation point.
- `BrowseAsyncUnknownNodeReturnsNullContinuationPointAsync` — an unknown
node returns `BadNodeIdUnknown` and a null continuation point.

## Not ported
The original commit also touched files that do not exist on master378:
- `samples/PumpDeviceIntegrationServer/Dockerfile` (unrelated
diagnostics-socket change; sample not present)
- The v2.0-era test files (`BrowseContinuationPointTests.cs`,
`BrowseTests.cs`, `MasterNodeManagerDeterministicTests.cs`) that were
added in the v2.0 refactor and don't exist on master378.

## Validation
- `dotnet build Libraries/Opc.Ua.Server` (net8.0, Release): succeeded, 0
warnings.
- `dotnet test Tests/Opc.Ua.Server.Tests --filter
MasterNodeManagerBrowseTests` (net8.0): 2 passed.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

---------

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…prevent server-start NullReferenceException (backport to master378) (#4264)

Backport of
9883c41
(#4055) from `master` to `master378`.

## Description

`ValidateAsync(ApplicationType.Server)` accepted a null
`TransportQuotas` without error, but the transport/secure-channel layers
assume it is non-null, so `ApplicationInstance.StartAsync` later threw a
`NullReferenceException`. The fluent builder path
(`ApplicationInstance.Build`) already always initializes
`TransportQuotas = new TransportQuotas()`; manually-assembled
configurations could leave it null.

Changes:

- **`ApplicationConfiguration.ValidateAsync`**: default
`TransportQuotas` when null, aligning the manual-construction path with
the builder path.

  ```csharp
// Transport/secure-channel layers rely on this being non-null during
bring-up.
  TransportQuotas ??= new TransportQuotas();
  ```

`TransportQuotas`'s default constructor yields complete, valid limits
(`DefaultEncodingLimits` / `TcpMessageLimits`), so minimal server
configs now start with sensible defaults instead of crashing.

- **Tests** (`ApplicationConfigurationTests`, new file on this branch):
`ValidateAsyncDefaultsTransportQuotasWhenNull`,
`ValidateAsyncKeepsExplicitTransportQuotas`, and
`ValidateAsyncDefaultsTransportQuotasWhenNullForClient`.

## Notes

- Ported to `master378`. The test fixture did not exist on this branch,
so it was added.
- Built `Opc.Ua.Core.Tests` (net8.0) with 0 warnings; the 3 new tests
pass.

## Checklist

- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
)

## Summary
Backports the fix for ServerTimestamp updating on Read operations for
stored/static nodes to master378 (1.5.378), addressing issue #4257 (part
2).

## Details
- In CustomNodeManager and CoreNodeManager, ensure ServerTimestamp is
stamped with the read time for stored/static nodes whose SourceTimestamp
is older than eadTime (e.g. after a Write).
- Retain matching SourceTimestamp and ServerTimestamp for dynamically
produced values generated during Read (such as ServerStatus children).
- Adds unit test
ReadStaticVariableStampsFreshServerTimestampAfterWriteAsync in
ReferenceServerTest.cs to verify that after writing to a static
variable, subsequent reads return ServerTimestamp stamped with the
current read time.

Fixes #4257 (ServerTimestamp behavior)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…#4341)

# Description

Fixes the reverse connect problem reported in #3985: a Client which uses
one reverse connect listener port for several Servers connects to the
first Server, and every following `WaitForConnectionAsync` times out. A
separate listener port per Server was the only workaround.

## Root cause

Confirming the analysis in [mrsuciu's comment on
#3985](#3985):

- Servers which send `ReverseHello` before the application registered a
waiting connection for them are held in `OnConnectionWaitingAsync` for
`HoldTime`.
- All held connections share a single `CancellationTokenSource`, so
`RegisterWaitingConnection` wakes up **every** held connection, not only
the one it was registered for.
- The hold loop was `while (!matched) { ... break; }` with an
unconditional `break`, so each woken connection got exactly one re-match
attempt. The connections which did not match the new registration fell
out of the loop unaccepted, although most of their own hold time was
left.
- An unaccepted connection is force faulted by the listener
(`TcpReverseConnectChannel`, "The reverse connection was rejected by the
client"), so that Server is dropped and only returns on its next
`ReverseHello` interval, long after the Client timed out waiting for it.

Separate ports avoid the shared wakeup path, which is exactly why that
workaround succeeds.

## Changes

- `ReverseConnectManager.OnConnectionWaitingAsync` re-arms the hold
after a wakeup: it re-matches, and if the registration which caused the
wakeup was for another Server the connection keeps waiting for the
remainder of its own hold time. The loop is still bounded by `HoldTime`,
so a Server which is never registered for is rejected exactly as before.
- A shutdown flag set by `StopService`/`Dispose` (cleared by
`StartService`) releases held connections immediately during teardown,
instead of parking them until the hold time expires. `Dispose` now also
cancels the token source before disposing it, so a held callback can no
longer read a disposed token.
- `Docs/ReverseConnect.md`: new "Sharing a listener across multiple
Servers" section documenting that the listener port staying in
`LISTENING` is expected, that one shared `ReverseConnectManager` should
serve all Servers on the same Client Url, and the
`HoldTime`/`WaitTimeout` semantics.

No public API, signature or serialization change.

## Validation

- New `ReverseConnectManagerUnitTests` (3 tests): the two Server
regression, hold time expiry still rejecting, and dispose releasing a
held connection. They drive the callback through an internal test hook,
so no listener or socket is involved. Verified that the regression test
fails against the previous behaviour with "The reverse connection of the
second server was released before its hold time expired."
- New tests pass on net10.0 and net48.
- Full reverse connect suite (`FullyQualifiedName~ReverseConnect`,
including the existing `ReverseConnectTest` server fixture): 33/33 pass
on net10.0.

## Related Issues

- Relates to #3985.
- `master` carries the identical defect: the same `break` survived #4009
and #4059 and is still present in
`src/Opc.Ua.Client/ReverseConnectManager.cs`. The equivalent fix for
`master` is #4342 (it uses `TimeProvider` and breaks on a pending
callback drain instead of a new flag), so the issue is intentionally not
auto closed here.

## Checklist

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] 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.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
# Description

Fixes server side handling of PublishRequests with maximum TimeoutHint
on master378 branch.

## Related Issues


- Fixes master378 coresponding 4371 issue

## Checklist

_Put an `x` in the boxes that apply. You can complete these step by step
after opening the PR._

- [ ] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
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.
…cific Retain = false (#4454)

# Description

Backport of #4453 to the 1.5.378 line.

When `SupportsFilteredRetain` is true and a condition drops out of a
client's where clause, the server sends one trailing event to that
client. Before this change the trailing event carried the condition's
real `Retain` value, typically `true`, so a spec-following client kept
an alarm on its display that it should have dropped. OPC 10000-9, 5.5.2
(Figure 11) and the "Retain sent" column of Table B.3 require that event
to be sent with a client specific `Retain = false`.

The value is per client and per filter, so it is applied inside the
monitored item:

* `CanSendFilteredAlarm` gains an `out bool overrideRetain` parameter
that is `true` only for the trailing out-of-scope event. The protected
signature is changed in place rather than overloaded.
* For that event the event fields are read through a new internal
`FilteredRetainTarget` decorator. It delegates everything to the
original `IFilterTarget` and returns `false` for a `Retain` value clause
the target actually resolves. A clause the type check rejects stays null
so the field list keeps its shape.
* The shared `InstanceStateSnapshot` that `ReportEvent` fans out to
every monitored item is never touched, so other subscriptions whose
filter the condition still passes keep receiving the server's real
value. The queued `EventFieldList` keeps the original instance as its
`Handle`, so reference-based duplicate detection and node manager handle
lookups are unaffected.

Differences from the master change: this branch has neither the #4377
filtered-retain rework nor `docs/AlarmsAndConditions.md`, so the
decorator is written against the `object`-based `IFilterTarget` of this
line, the existing tracking logic is left as it is, and there is no
documentation change. The reflection-based test helper reads the new out
parameter back from the argument array.

Tests added in `FilterRetainTests`:

* the step-5 event from the issue queued through two monitored items
sharing one snapshot: the out-of-scope client receives `Retain = false`,
the still-passing client receives `true`, and the snapshot itself still
reports `true`;
* the override applies to the single trailing event only and the server
value returns once the condition re-enters scope;
* an unresolvable `Retain` clause stays null;
* the existing 16-step Table B.3 test now also asserts the "Retain sent"
column for the trailing rows.

## Related Issues

- Fixes #4449 for 1.5.378
- Backport of #4453

## Checklist

- [x] I have signed the
[CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf)
and read the
[CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md)
doc.
- [x] I have added tests that prove my fix is effective or that my
feature works and increased code coverage.
- [x] I have added all necessary documentation.
- [x] 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.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@CLAassistant

CLAassistant commented Sep 8, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
4 out of 5 committers have signed the CLA.

✅ KarenKrill
✅ marcschier
✅ mrsuciu
✅ romanett
❌ Copilot
You have signed the CLA already but the status is still pending? Let us recheck it.

@codecov

codecov Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.27778% with 53 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.90%. Comparing base (52294d2) to head (c896c67).
⚠️ Report is 7 commits behind head on release/1.5.378.

Files with missing lines Patch % Lines
Libraries/Opc.Ua.Client/Session/Session.cs 67.44% 9 Missing and 5 partials ⚠️
.../Opc.Ua.Server/Subscription/SessionPublishQueue.cs 83.67% 7 Missing and 1 partial ⚠️
...ies/Opc.Ua.Server/NodeManager/MasterNodeManager.cs 73.68% 5 Missing ⚠️
...es/Opc.Ua.Server/Server/ConformanceUnitsManager.cs 92.64% 3 Missing and 2 partials ⚠️
Libraries/Opc.Ua.Client/ReverseConnectManager.cs 86.66% 4 Missing ⚠️
Libraries/Opc.Ua.Server/Server/StandardServer.cs 82.60% 4 Missing ⚠️
....Ua.Server/Aggregates/StdDevAggregateCalculator.cs 75.00% 3 Missing ⚠️
...aries/Opc.Ua.Server/NodeManager/CoreNodeManager.cs 0.00% 3 Missing ⚠️
...Subscription/MonitoredItem/FilteredRetainTarget.cs 88.88% 1 Missing and 1 partial ⚠️
...ack/Opc.Ua.Core/Stack/Tcp/ChannelAsyncOperation.cs 0.00% 1 Missing and 1 partial ⚠️
... and 3 more
Additional details and impacted files

Impacted file tree graph

@@                 Coverage Diff                 @@
##           release/1.5.378    #4457      +/-   ##
===================================================
+ Coverage            60.15%   61.90%   +1.74%     
===================================================
  Files                  378      380       +2     
  Lines                79069    79297     +228     
  Branches             13836    13884      +48     
===================================================
+ Hits                 47567    49090    +1523     
+ Misses               27084    25782    -1302     
- Partials              4418     4425       +7     
Files with missing lines Coverage Δ
Libraries/Opc.Ua.Client/Browser.cs 78.09% <100.00%> (+8.88%) ⬆️
...a.Configuration/ApplicationConfigurationBuilder.cs 73.55% <ø> (+0.06%) ⬆️
...raries/Opc.Ua.Configuration/ApplicationInstance.cs 62.64% <100.00%> (+0.14%) ⬆️
...es/Opc.Ua.Server/Aggregates/AggregateCalculator.cs 83.94% <100.00%> (+57.18%) ⬆️
...c.Ua.Server/Aggregates/CountAggregateCalculator.cs 86.36% <100.00%> (+86.36%) ⬆️
....Ua.Server/Aggregates/MinMaxAggregateCalculator.cs 79.62% <100.00%> (+79.62%) ⬆️
....Ua.Server/Aggregates/StatusAggregateCalculator.cs 90.58% <100.00%> (+90.58%) ⬆️
Libraries/Opc.Ua.Server/Server/RequestManager.cs 62.35% <100.00%> (+0.44%) ⬆️
Libraries/Opc.Ua.Server/Session/Session.cs 62.45% <100.00%> (+2.17%) ⬆️
...Server/Subscription/MonitoredItem/MonitoredItem.cs 64.19% <100.00%> (+1.13%) ⬆️
... and 18 more

... and 23 files with indirect coverage changes

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

@marcschier marcschier added the 1.5.378 Only affects 1.5.378 (pre 2.0) label Sep 9, 2026
@marcschier marcschier changed the title Merge fixes from master to Release/1.5.378 Merge fixes from master378 to Release/1.5.378 Sep 9, 2026
# Description

Backport CTT related changes from master into master378.
…s.Git from 10.0.102 to 10.0.111 (#4465)

Update Microsoft.SourceLink.GitHub and
Microsoft.SourceLink.AzureRepos.Git from 10.0.102 to 10.0.111
@mrsuciu
mrsuciu merged commit 2869e81 into release/1.5.378 Sep 11, 2026
92 of 93 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

1.5.378 Only affects 1.5.378 (pre 2.0)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants