mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-27 04:59:55 +08:00
# Description Fixes defects found in a spec-grounded audit of the server **Subscription / MonitoredItem** implementation (`src/Opc.Ua.Server/Subscription/**`, `NodeManager/MonitoredItem/**`, `EventManager`, monitored-item service dispatch and filter validation). Each behaviour change was checked against OPC 10000-4 (§5.13, §5.14, §7.21, §7.22), Part 5 (subscription diagnostics) and Part 9 (ConditionRefresh). Every fix has a regression test. The audit found 56 defects (1 Critical, 3 High, 20 Medium, 32 Low). A follow-up max-effort code review of the fix branch found 27 more issues, mostly introduced by the first round of fixes. All but one are fixed here. ### Most important fixes - **Critical — remote hang:** an `AggregateFilter` with `processingInterval = NaN` was not revised, and `AggregateCalculator` looped forever while holding the node-manager lock, so one CreateMonitoredItems request blocked the node manager. Non-finite, ≤0 and sub-tick intervals are now revised (Part 4 §5.13.2.1), and the calculator rejects them as a backstop. - **Monitored-item limits:** new `ServerConfiguration.MaxMonitoredItemCount` / `MaxMonitoredItemsPerSubscription`; Bad_TooManyMonitoredItems is returned when a limit is hit, and ServerCapabilities now report the limits. Both default to `0` (no limit), so existing behaviour is unchanged. - **Sampling groups:** a client could get one dedicated thread per distinct sampling interval above 1 h; intervals above the last rate group are now rounded, and the loop is async. An item could also end up in two sampling groups and keep being sampled after it was deleted. - **Subscription state machine** (Part 4 §5.14.1.2): - the lifetime counter resets while a Publish is queued and counts otherwise; - the lifetime and keep-alive stay within `MaxSubscriptionLifetime`; - LatePublishRequestCount is counted once per entry into LATE; - messages held while publishing is disabled are no longer sent, and are not offered by Transfer/Republish either; - ConditionRefresh returns Bad_RefreshInProgress for any refresh already queued or running on the subscription. - **Triggering** (Part 4 §5.13.1.6): - linked items are triggered by an item still draining a backlog, by the first value after re-enable, and by an item created Disabled; - links are persisted for durable subscriptions; - Disabled/Sampling items follow the publish rules in the shared pipeline, so custom `IMonitoredItem` implementations follow them too. - **Queues** (Part 4 §5.13.1.5, §7.21): - no duplicate value after a queue-size increase; - EventQueueOverflow events are placed where the events were lost, including on queue shrink; - the duplicate-event check scans the newest entries; - EventQueueOverflowCount and MonitoringQueueOverflowCount are maintained correctly; - changing select clauses rebuilds queued events in the new layout, and is safe on durable queues and on failure. - **Filters:** - per-clause EventFilterResult instead of rejecting the whole item (Part 4 §7.22.3); - corrected status codes: Bad_DeadbandFilterInvalid, Bad_MonitoredItemFilterInvalid, Bad_EventFilterInvalid, Bad_MonitoringModeInvalid, Bad_TimestampsToReturnInvalid; - aggregate filter in subscriptions: processing interval at least 2× the revised sampling interval, revised StartTime on the requested grid, ServerTimestamp = end of interval (Part 4 §7.22.4). - **Lifecycle and races:** - expired subscriptions are always cleaned up, even when the session closes during cleanup; - Create/Transfer racing CloseSession returns Bad_SessionClosed; - session diagnostics counts no longer drift; - a large but valid Publish TimeoutHint no longer fails the request; - a ready subscription is served by queued Publish requests instead of going Late; - restored persisted state is validated. - **Access:** a value change notification is checked against each subscriber's UserAccessLevel, not the writer's. - **Redundancy / durable store:** - the first-unsent sequence number is mirrored; - sequence numbers are no longer reused after a second failover; - triggering links are stored (definition format 6, written only when links exist, so older replicas can still read link-less subscriptions). ### Behaviour changes reviewers should know - With the default config, publishing intervals above `MaxSubscriptionLifetime / 3` (1,200,000 ms) are revised down, so lifetime ≥ 3×keep-alive fits the configured maximum. - The revised keep-alive is capped at a third of the max lifetime. - Idle subscriptions expire after exactly MaxLifetimeCount cycles without a queued Publish. - A partly invalid EventFilter select list is accepted with per-clause errors. The public `EventFilter.Validate` keeps Status Good if at least one clause is valid; callers that cannot return per-clause results must also check the new `EventFilter.Result.HasSelectClauseErrors`. There is a migration note in `docs/migrate/2.0.x/sessions-subscriptions.md`. - Aggregate processing intervals may be revised higher (≥ 2× sampling interval); aggregate ServerTimestamp is now the interval end. - Sampling intervals above 3,600,000 ms are rounded to multiples of 900,000 ms (default rate groups). ### Public API Additive only: - `ServerConfiguration.MaxMonitoredItemCount` and `MaxMonitoredItemsPerSubscription`; - `EventFilter.Result.HasSelectClauseErrors`; - `IStoredSubscriptionTriggering`, `ISubscriptionRetransmissionSendStateStore`, `SubscriptionRetransmissionState.FirstUnsentSequenceNumber` (optional store capabilities, detected with `as`); - new `EventQueueHandler` constructor overloads. ### Known open items (not in this PR) - **Part 4 §5.13.2.1:** CreateMonitoredItems should create an item even when the user lacks Read permission and report the error in Publish. This is not done: the create-time RolePermissions check is the only role check on the initial value, so following the spec would leak values until every node manager re-checks per sample. - **Sampling loop:** it runs on thread-pool threads; node managers with blocking synchronous reads should not use sampling groups heavily. The alternative, a dedicated thread per group, was the thread-exhaustion issue fixed here. - **Restarts:** a pending QueueSize-1 value and a pending overflow flag are not kept across a restart; that needs persistence interface extensions. - **Limit defaults:** decide whether the new limits should get finite defaults (currently 0 = unlimited). ## Related Issues - No tracking issue; the findings come from an internal audit of the server subscription code. ## Checklist - [ ] 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. - [ ] 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. ### Local verification (net10.0, all green) | Suite | Passed | Failed | |---|---|---| | Opc.Ua.Server.Tests | 7963 | 0 | | Opc.Ua.Subscriptions.Tests | 643 | 0 | | Opc.Ua.Subscriptions.Classic.Tests | 57 | 0 | | Opc.Ua.Subscriptions.Durable.Tests | 36 | 0 | | Opc.Ua.Redundancy.Server.Tests | 847 | 0 | | Opc.Ua.Core.Tests (filter tests) | 283 | 0 | net48 build of `Opc.Ua.Server.Tests`: 0 errors. net48 tests and the full UA.slnx were not run locally. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Marc Schier <marcschier@users.noreply.github.com>
201 lines
8.8 KiB
C#
201 lines
8.8 KiB
C#
/* ========================================================================
|
|
* Copyright (c) 2005-2026 The OPC Foundation, Inc. All rights reserved.
|
|
*
|
|
* OPC Foundation MIT License 1.00
|
|
*
|
|
* Permission is hereby granted, free of charge, to any person
|
|
* obtaining a copy of this software and associated documentation
|
|
* files (the "Software"), to deal in the Software without
|
|
* restriction, including without limitation the rights to use,
|
|
* copy, modify, merge, publish, distribute, sublicense, and/or sell
|
|
* copies of the Software, and to permit persons to whom the
|
|
* Software is furnished to do so, subject to the following
|
|
* conditions:
|
|
*
|
|
* The above copyright notice and this permission notice shall be
|
|
* included in all copies or substantial portions of the Software.
|
|
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
|
|
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
|
|
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
|
|
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
|
|
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
|
|
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
|
|
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
|
|
* OTHER DEALINGS IN THE SOFTWARE.
|
|
*
|
|
* The complete license agreement can be found here:
|
|
* http://opcfoundation.org/License/MIT/1.00/
|
|
* ======================================================================*/
|
|
|
|
using Moq;
|
|
using NUnit.Framework;
|
|
using Opc.Ua.Server.Tests.NodeManager;
|
|
|
|
namespace Opc.Ua.Server.Tests
|
|
{
|
|
/// <summary>
|
|
/// Verifies durable lifetime-unit conversion and publish errors for sessions without a publish queue.
|
|
/// </summary>
|
|
[TestFixture]
|
|
[Category("Subscription")]
|
|
public sealed class DurableKeepAliveRegressionTests
|
|
{
|
|
/// <summary>
|
|
/// Verifies that durable keep-alive limits convert configured hours to milliseconds before revising counts.
|
|
/// </summary>
|
|
[TestCase(1, 1000.0, 0u, 3u)]
|
|
[TestCase(1, 1000.0, 10u, 10u)]
|
|
[TestCase(1, 1000.0, 1199u, 1199u)]
|
|
[TestCase(1, 1000.0, 1200u, 1200u)]
|
|
[TestCase(1, 1000.0, 1201u, 1200u)]
|
|
[TestCase(1, 1000.0, 3601u, 1200u)]
|
|
[TestCase(24, 1000.0, 10u, 10u)]
|
|
[TestCase(2000, 1000.0, 10u, 10u)]
|
|
[TestCase(int.MaxValue, 1000.0, 10u, 10u)]
|
|
public void DurableKeepAliveUsesMillisecondsInsteadOfHours(
|
|
int hours, double interval, uint requested, uint expected)
|
|
{
|
|
Mock<IServerInternal> server = DeterministicServerMock.Create(out MonitoredItemQueueFactory queues);
|
|
using (queues)
|
|
using (var manager = new RevisionHooks(server.Object, hours))
|
|
{
|
|
Assert.That(manager.KeepAlive(interval, requested, true), Is.EqualTo(expected));
|
|
Assert.That(manager.KeepAlive(interval, 10, false), Is.EqualTo(10));
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Verifies that the keep-alive is capped at a third of the maximum lifetime, so the
|
|
/// lifetime raised to three keep-alive intervals stays within the configured maximum.
|
|
/// </summary>
|
|
[TestCase(1000.0, 10u, 10u)]
|
|
[TestCase(1000.0, 20u, 20u)]
|
|
[TestCase(1000.0, 21u, 20u)]
|
|
[TestCase(1000.0, 10000u, 20u)]
|
|
[TestCase(7000.0, 10u, 2u)]
|
|
[TestCase(30000.0, 10u, 1u)]
|
|
public void KeepAliveIsCappedSoTheRevisedLifetimeStaysWithinTheMaximum(
|
|
double interval, uint requested, uint expected)
|
|
{
|
|
Mock<IServerInternal> server = DeterministicServerMock.Create(out MonitoredItemQueueFactory queues);
|
|
using (queues)
|
|
using (var manager = new RevisionHooks(server.Object, 24))
|
|
{
|
|
uint keepAlive = manager.KeepAlive(interval, requested, false);
|
|
Assert.That(keepAlive, Is.EqualTo(expected));
|
|
uint lifetime = manager.OrdinaryLifetime(interval, keepAlive, 0);
|
|
Assert.That(lifetime, Is.EqualTo(3 * keepAlive));
|
|
if (interval * 3 <= 60000)
|
|
{
|
|
Assert.That(lifetime * interval, Is.LessThanOrEqualTo(60000));
|
|
}
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Verifies that long durable lifetimes are capped correctly without overflowing a 32-bit millisecond value.
|
|
/// </summary>
|
|
[Test]
|
|
[TestCase(1193, 1.0, 4_294_800_000u)]
|
|
[TestCase(1194, 1.0, uint.MaxValue)]
|
|
public void DurableLifetimeConversionDoesNotOverflowUInt32Milliseconds(
|
|
int hours,
|
|
double interval,
|
|
uint expected)
|
|
{
|
|
Mock<IServerInternal> server = DeterministicServerMock.Create(out MonitoredItemQueueFactory queues);
|
|
using (queues)
|
|
using (var manager = new RevisionHooks(server.Object, hours))
|
|
{
|
|
Assert.That(manager.Lifetime(interval, 1, uint.MaxValue), Is.EqualTo(expected));
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Verifies the existing 2000-hour lifetime boundary remains exact.
|
|
/// </summary>
|
|
[Test]
|
|
public void DurableLifetimeConversionPreservesExistingBoundary()
|
|
{
|
|
Mock<IServerInternal> server = DeterministicServerMock.Create(out MonitoredItemQueueFactory queues);
|
|
using (queues)
|
|
using (var manager = new RevisionHooks(server.Object, 2000))
|
|
{
|
|
Assert.That(manager.Lifetime(1000, 1, 7_199_999), Is.EqualTo(7_199_999));
|
|
Assert.That(manager.Lifetime(1000, 1, 7_200_000), Is.EqualTo(7_200_000));
|
|
Assert.That(manager.Lifetime(1000, 1, 7_200_001), Is.EqualTo(7_200_000));
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Verifies that a missing publish queue distinguishes a closing session from one without subscriptions.
|
|
/// </summary>
|
|
[TestCase(false)]
|
|
[TestCase(true)]
|
|
public void PublishMissingQueueKeepsClosingSessionDistinct(bool closing)
|
|
{
|
|
Mock<IServerInternal> server = DeterministicServerMock.Create(out MonitoredItemQueueFactory queues);
|
|
using (queues)
|
|
using (var manager = new RevisionHooks(server.Object, 24))
|
|
{
|
|
var session = new Mock<ISession>();
|
|
session.SetupGet(value => value.Id).Returns(new NodeId(1, 1));
|
|
session.SetupGet(value => value.IsClosing).Returns(closing);
|
|
using var context = new OperationContext(session.Object, DiagnosticsMasks.None);
|
|
ServiceResultException error = Assert.ThrowsAsync<ServiceResultException>(async () =>
|
|
await manager.PublishAsync(context, [], null).ConfigureAwait(false));
|
|
Assert.That(error.StatusCode,
|
|
Is.EqualTo(closing ? StatusCodes.BadSessionClosed : StatusCodes.BadNoSubscription));
|
|
}
|
|
}
|
|
|
|
/// <summary>
|
|
/// Exposes subscription count revision using a configurable maximum durable lifetime.
|
|
/// </summary>
|
|
private sealed class RevisionHooks : SubscriptionManager
|
|
{
|
|
/// <summary>
|
|
/// Creates a manager with durable subscriptions enabled and the requested lifetime limit in hours.
|
|
/// </summary>
|
|
public RevisionHooks(IServerInternal server, int hours)
|
|
: base(server, new ApplicationConfiguration
|
|
{
|
|
ServerConfiguration = new ServerConfiguration
|
|
{
|
|
MaxDurableSubscriptionLifetimeInHours = hours,
|
|
MaxPublishingInterval = int.MaxValue,
|
|
MaxSubscriptionLifetime = 60000,
|
|
MinSubscriptionLifetime = 0,
|
|
DurableSubscriptionsEnabled = true
|
|
}
|
|
})
|
|
{
|
|
}
|
|
|
|
/// <summary>
|
|
/// Revises a keep-alive count for the supplied publishing interval and durability mode.
|
|
/// </summary>
|
|
public uint KeepAlive(double interval, uint count, bool durable)
|
|
{
|
|
return CalculateKeepAliveCount(interval, count, durable);
|
|
}
|
|
|
|
/// <summary>
|
|
/// Revises a durable lifetime count using the supplied publishing interval and keep-alive count.
|
|
/// </summary>
|
|
public uint Lifetime(double interval, uint keepAlive, uint count)
|
|
{
|
|
return CalculateLifetimeCount(interval, keepAlive, count, true);
|
|
}
|
|
|
|
/// <summary>
|
|
/// Revises an ordinary lifetime count using the supplied publishing interval and keep-alive count.
|
|
/// </summary>
|
|
public uint OrdinaryLifetime(double interval, uint keepAlive, uint count)
|
|
{
|
|
return CalculateLifetimeCount(interval, keepAlive, count, false);
|
|
}
|
|
}
|
|
}
|
|
}
|