Links to a deleted MonitoredItem were only removed when the triggering item reported. If it never reports (Disabled mode, or a value that does not change), they stay forever, and since MonitoredItem ids are never reused a client can grow the link array of one item without bound by creating, linking and deleting items in a loop. Every addLink scans that array, so SetTriggering gets slower with each round.
Part 4, 5.13.1.6 says that the link goes away with the triggered item. SetTriggering now prunes the links of the triggering item to MonitoredItems that no longer exist before applying the request, which keeps the array bounded by the items of the Subscription.
CreateMonitoredItems, ModifyMonitoredItems, SetMonitoringMode and DeleteMonitoredItems all check maxMonitoredItemsPerCall, but SetTriggering accepted any number of links. Apply the same limit there, counting the links to add and the links to remove together, and return Bad_TooManyOperations above it (Part 4, 5.13.5).
Also free removeResults with its own size in the out-of-memory path, it was using linksToAddSize.
Session and subscription diagnostics callbacks copied a node BrowseName into a fixed 128-byte stack buffer without checking its length. A modified diagnostics model could therefore overflow the stack when the node was read.
Reject names that cannot fit, including the terminator, and cover both callbacks with a regression test.
The per-node MonitoredItem back-pointer list (UA_NodeHead.monitoredItems)
was a hand-rolled singly-linked list. Removing an item scanned the list to
find its predecessor, so tearing down all MonitoredItems attached to one
node was O(n^2).
Switch the list to the intrusive BSD-style LIST_* macros from
deps/open62541_queue.h, already used for the subscription sampling list.
Removal follows the le_prev back-pointer and is O(1); no search is needed.
The list head stays the bare UA_MonitoredItem* in the public UA_NodeHead
(plugin/nodestore.h): it is layout-compatible with a LIST_HEAD, so the
server casts the address of that field to UA_MonitoredItemList* to run the
LIST_* macros over it. The public header and the generated amalgamation
therefore stay independent of open62541_queue.h. The cast is type-punning
that relies on this layout compatibility, and the build already sets
-fno-strict-aliasing.
The intrusive list adds an invariant the previous one did not have: the
le_prev of the first element points into the node that holds the list
head. Nothing has to be re-anchored on a node copy, because the server
only edits nodes in place -- UA_Server_editNode goes through getEditNode,
which the ziptree nodestore implements in-situ -- so a live node never
moves. A nodestore that instead relocates a node (getNodeCopy followed by
replaceNode, which no caller in the server uses today) has to re-anchor
le_prev onto the replacement after a successful replace: carrying the head
pointer over on its own is not sufficient. Re-anchoring at copy time would
be wrong, since the copy still shares the list with the source and the
replace may still fail.
Add a regression test on the per-node list (samplingInterval == 0, "on
write" sampling) covering head, interior and tail removal as well as the
on-write iterator, asserting the *le_prev == element invariant on every
element throughout.
Tearing down N MonitoredItems attached to a single node was O(n^2).
Deleting them now scales linearly:
N delete before delete after
20000 669 ms 29 ms (x23)
50000 6477 ms 71 ms (x92)
80000 25756 ms 120 ms (x215)
100000 66072 ms 169 ms (x391)
(amalgamation of each revision built identically with -O2 -DNDEBUG.)
Closing the owning session from MONITOREDITEM_CREATED queues MonitoredItem, Subscription and Session cleanup in one reentrant call stack. Verify that current 1.5 lifecycle handling removes all three objects and that the later Subscription and Session cleanup callbacks are each reached exactly once.
Make MonitoredItem and Subscription removal terminal and idempotent while their allocations remain delayed-valid. Prevent sampling, data-change processing, and event-filter completion from enqueueing work after reentrant deletion.
Cover deletion from an application-backed data-source read and Session closure from a MonitoredItem deletion callback during Subscription teardown.
Honor the established zero-as-unlimited convention when creating Sessions and SecureChannels.
Document the sentinel behavior and add regression coverage for both paths.
When a MonitoredItem with SamplingInterval=0 targets a DataSource node,
the server was unconditionally installing an EVENT (backpointer) sampler.
Backpointer sampling only fires when the node is written via the OPC UA
Write service. DataSource nodes (e.g. Server/ServerStatus/CurrentTime)
compute their value on-the-fly in a read callback; they are never written
via Write, so no DataChangeNotification was ever enqueued.
A regression test is added to check_services_subscriptions.c that monitors
Server/ServerStatus/CurrentTime (i=2258) -- always available, no compile
flags required. The test verifies:
- samplingType == UA_MONITOREDITEMSAMPLINGTYPE_PUBLISH (structural fix)
- queue grows after each publish cycle (functional fix)
- second notification has a strictly later DateTime value
Fix a build failure with recent GCC versions when compiling with
`-Werror`.
The function `notifySubscription()` stores the address of a stack
variable (`enabled`) in a `UA_Variant` via `UA_Variant_setScalar()`.
Newer GCC versions detect this as a potential dangling pointer
(`-Wdangling-pointer`).
To avoid storing a pointer to a stack variable, the value is now taken
directly from the subscription object instead of using a temporary local
variable.
error: storing the address of local variable 'enabled' in
'createSubData[7].value.data' [-Werror=dangling-pointer]
- Remove the temporary stack variable `enabled`
- Use the subscription state directly when populating the `UA_Variant`
``` c
UA_Boolean enabled = (sub->state = UA_SUBSCRIPTIONSTATE_ENABLED);
UA_Variant_setScalar(&createSubData[7].value, &enabled,
&UA_TYPES[UA_TYPES_BOOLEAN]);
```
``` c
sub->enabled = (sub->state == UA_SUBSCRIPTIONSTATE_ENABLED);
UA_Variant_setScalar(&createSubData[7].value, &sub->enabled,
&UA_TYPES[UA_TYPES_BOOLEAN]);
```
- OS: Linux
- Compiler: GCC 13 / GCC 14
- Build flags: `-Wall -Wextra -Werror`
fix(server): avoid dangling pointer warning in notifySubscription
Fix a build failure with recent GCC versions when compiling with
`-Werror`.
The function `notifySubscription()` stores the address of a stack
variable (`enabled`) in a `UA_Variant` via `UA_Variant_setScalar()`.
Newer GCC versions detect this as a potential dangling pointer
(`-Wdangling-pointer`).
To avoid storing a pointer to a stack variable, the value is now taken
directly from the subscription object instead of using a temporary local
variable.
error: storing the address of local variable 'enabled' in
'createSubData[7].value.data' [-Werror=dangling-pointer]
- Remove the temporary stack variable `enabled`
- Use the subscription state directly when populating the `UA_Variant`
``` c
UA_Boolean enabled = (sub->state = UA_SUBSCRIPTIONSTATE_ENABLED);
UA_Variant_setScalar(&createSubData[7].value, &enabled,
&UA_TYPES[UA_TYPES_BOOLEAN]);
```
``` c
sub->enabled = (sub->state == UA_SUBSCRIPTIONSTATE_ENABLED);
UA_Variant_setScalar(&createSubData[7].value, &sub->enabled,
&UA_TYPES[UA_TYPES_BOOLEAN]);
```
- OS: Linux
- Compiler: GCC 13 / GCC 14
- Build flags: `-Wall -Wextra -Werror`
server: fix potential dangling pointer in subscription notification data and add test
When a session timed out, the server unconditionally deleted all
subscriptions owned by that session. Per OPC UA Part 4, §5.13.7,
the subscription lifetime is independent of the session lifetime.
Subscriptions whose lifetime has not expired must survive the session
timeout so clients can recover them via TransferSubscriptions.
Detach subscriptions (instead of deleting them) when the shutdown
reason is UA_SHUTDOWNREASON_TIMEOUT. Detached subscriptions remain
in the server-wide list with session==NULL and self-destruct when
their own lifetime expires.
- Modified allowTransferSubscription_default() to use UA_String_equal()
for direct string comparison instead of UA_order()
- Added explicit check: userId->length == 0 means anonymous user
- Anonymous users are rejected from transferring subscriptions
- Added Server_transferSubscription_anonymous unit test
- Added createAuthenticatedSession() helper for authenticated tests
- Updated Server_transferSubscriptionDiagnostics to use authenticated sessions
- All 17 unit tests pass
This fix addresses the OPC UA CTT compliance requirement that
anonymous users should not be allowed to transfer subscriptions.
Add wasTransferred flag to UA_Subscription to prevent incorrect
currentSubscriptionCount decrements when a subscription is transferred
to another session and the old subscription object is deleted.
When a subscription is transferred, the old subscription object is
deleted but the subscription continues to exist under a new object.
The diagnostics counter should only be decremented when a subscription
is truly deleted, not when it's being replaced during a transfer.
Includes test case to verify the fix.
add author information
Niels Beier