Skip to content

Commit 8e263a0

Browse files
committed
fix(sort): skip non-hierarchical refs in topological sort
The topological sort in Nodeset_sort() determines the order in which nodes are added to the server. Previously, nodeRefsReady() treated ALL inverse references (except HasTypeDefinition) as ordering dependencies. This caused infinite-loop deadlocks when two nodes in the same nodeset referenced each other via a non-hierarchical reference type. Only inverse hierarchical references (subtypes of HierarchicalReferences i=33) create true parent-child ordering dependencies. Non-hierarchical references (e.g. HasCondition, HasSubStateMachine) are cross-references that do not require ordering. Instead of hardcoding known hierarchical reference type IDs, this fix introduces an isHierarchicalRef callback in NL_FileContext. The open62541 backend implements this callback by using UA_Server_browseRecursive to discover all subtypes of HierarchicalReferences from the server's reference type hierarchy at init time. This makes the check robust against future OPC UA spec additions and custom hierarchical reference types from any namespace.
1 parent bcfe420 commit 8e263a0

4 files changed

Lines changed: 88 additions & 4 deletions

File tree

backends/open62541/src/import.c

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,13 +90,36 @@ AddNodeContext_init(AddNodeContext *ctx,
9090
UA_Array_append((void**)&ctx->parentRefTypes,
9191
&ctx->parentRefTypesSize, &hasChildExp,
9292
&UA_TYPES[UA_TYPES_EXPANDEDNODEID]);
93+
94+
// Get all hierarchical ReferenceTypes (subtypes of HierarchicalReferences).
95+
// Used by the topological sort to distinguish ordering dependencies
96+
// from non-hierarchical cross-references.
97+
UA_BrowseDescription bd2;
98+
UA_BrowseDescription_init(&bd2);
99+
bd2.browseDirection = UA_BROWSEDIRECTION_FORWARD;
100+
bd2.referenceTypeId = UA_NS0ID(HASSUBTYPE);
101+
bd2.nodeId = UA_NS0ID(HIERARCHICALREFERENCES);
102+
103+
UA_Server_browseRecursive(server, &bd2,
104+
&ctx->hierarchicalRefTypesSize,
105+
&ctx->hierarchicalRefTypes);
106+
107+
// Include HierarchicalReferences itself
108+
UA_ExpandedNodeId hierRefExp;
109+
UA_ExpandedNodeId_init(&hierRefExp);
110+
hierRefExp.nodeId = UA_NS0ID(HIERARCHICALREFERENCES);
111+
UA_Array_append((void**)&ctx->hierarchicalRefTypes,
112+
&ctx->hierarchicalRefTypesSize, &hierRefExp,
113+
&UA_TYPES[UA_TYPES_EXPANDEDNODEID]);
93114
}
94115

95116
static void
96117
AddNodeContext_clear(AddNodeContext *ctx) {
97118
UA_NamespaceMapping_clear(&ctx->nsMapping);
98119
UA_Array_delete(ctx->parentRefTypes, ctx->parentRefTypesSize,
99120
&UA_TYPES[UA_TYPES_EXPANDEDNODEID]);
121+
UA_Array_delete(ctx->hierarchicalRefTypes, ctx->hierarchicalRefTypesSize,
122+
&UA_TYPES[UA_TYPES_EXPANDEDNODEID]);
100123
}
101124

102125
static inline UA_Boolean isValTrue(const char *s) {
@@ -497,6 +520,20 @@ addNodes(NodesetLoader *loader, AddNodeContext *anc) {
497520
return true;
498521
}
499522

523+
/* Callback for the topological sort: check whether a reference type is
524+
* hierarchical (subtype of HierarchicalReferences i=33). Only hierarchical
525+
* inverse references create parent-child ordering dependencies. */
526+
static bool
527+
isHierarchicalRef(void *userContext, const UA_NodeId *refType) {
528+
AddNodeContext *ctx = (AddNodeContext *)userContext;
529+
for(size_t i = 0; i < ctx->hierarchicalRefTypesSize; i++) {
530+
if(UA_NodeId_equal(refType,
531+
&ctx->hierarchicalRefTypes[i].nodeId))
532+
return true;
533+
}
534+
return false;
535+
}
536+
500537
bool
501538
NodesetLoader_loadFile(struct UA_Server *server, const char *path,
502539
NodesetLoader_ExtensionInterface *extensionHandling) {
@@ -524,6 +561,7 @@ NodesetLoader_loadFile(struct UA_Server *server, const char *path,
524561
handler.file = path;
525562
handler.extensionHandling = extensionHandling;
526563
handler.nsMapping = &ctx.nsMapping; // Provide the pre-filled mapping
564+
handler.isHierarchicalRef = isHierarchicalRef;
527565

528566
logger->log(logger->context, NODESETLOADER_LOGLEVEL_DEBUG,
529567
"Start import nodeset: %s", path);

backends/open62541/src/internal.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,12 @@ typedef struct {
2727
// Inherited from HasChild.
2828
size_t parentRefTypesSize;
2929
UA_ExpandedNodeId *parentRefTypes;
30+
31+
// All hierarchical ReferenceTypes (subtypes of HierarchicalReferences).
32+
// Used by the topological sort to distinguish ordering dependencies
33+
// from non-hierarchical cross-references.
34+
size_t hierarchicalRefTypesSize;
35+
UA_ExpandedNodeId *hierarchicalRefTypes;
3036
} AddNodeContext;
3137

3238
UA_NodeId

include/NodesetLoader/NodesetLoader.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,20 @@ typedef void (*NL_addNamespaceCallback)(void *userContext,
145145
UA_String *localNamespaceUris,
146146
UA_NamespaceMapping *nsMapping);
147147

148+
/* Callback to check if a reference type is hierarchical (subtype of
149+
* HierarchicalReferences i=33). Used by the topological sort to determine
150+
* ordering dependencies. If NULL, all inverse references are conservatively
151+
* treated as ordering dependencies. */
152+
typedef bool (*NL_isHierarchicalRefCallback)(void *userContext,
153+
const UA_NodeId *refType);
154+
148155
typedef struct NL_FileContext {
149156
void *userContext;
150157
const char *file;
151158
NL_addNamespaceCallback addNamespace;
152159
NodesetLoader_ExtensionInterface *extensionHandling;
153160
UA_NamespaceMapping *nsMapping;
161+
NL_isHierarchicalRefCallback isHierarchicalRef;
154162
} NL_FileContext;
155163

156164
struct NodesetLoader;

src/Nodeset.c

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,18 +136,50 @@ Nodeset_findByNodeId(Nodeset *nodeset, const UA_NodeId *key) {
136136

137137
static UA_NodeId hasTypeDef = {0, UA_NODEIDTYPE_NUMERIC, {40}};
138138

139+
/* Check whether a reference creates an ordering dependency for the
140+
* topological sort. Only two kinds of references require the target
141+
* to exist before the source node can be added:
142+
*
143+
* 1. Forward HasTypeDefinition -- the type node must exist first.
144+
* 2. Inverse hierarchical references -- the parent node must exist first.
145+
* For example, an Object with an inverse HasComponent to its parent
146+
* cannot be added before the parent exists.
147+
*
148+
* Non-hierarchical references (like HasCondition or HasSubStateMachine)
149+
* do NOT create ordering dependencies. If they are treated as such,
150+
* they cause deadlocks when two nodes in the same nodeset reference
151+
* each other via a non-hierarchical reference type.
152+
*
153+
* Whether a reference type is hierarchical is determined via the
154+
* isHierarchicalRef callback in the NL_FileContext. The open62541
155+
* backend sets this callback using UA_Server_browseRecursive to
156+
* discover all subtypes of HierarchicalReferences (i=33) from the
157+
* server's reference type hierarchy. If no callback is set, all
158+
* inverse references are conservatively treated as dependencies. */
139159
static bool
140-
nodeRefsReady(NL_Node *node) {
160+
nodeRefsReady(Nodeset *nodeset, NL_Node *node) {
161+
NL_isHierarchicalRefCallback isHierCb = NULL;
162+
void *userCtx = NULL;
163+
if(nodeset->fc) {
164+
isHierCb = nodeset->fc->isHierarchicalRef;
165+
userCtx = nodeset->fc->userContext;
166+
}
141167
for(NL_Reference *ref = node->refs; ref != NULL; ref = ref->next) {
142168
if(!ref->targetPtr)
143169
continue;
144170
if(ref->targetPtr->isDone)
145171
continue;
146172
if(UA_NodeId_equal(&hasTypeDef, &ref->refType)) {
173+
/* Forward HasTypeDefinition: the type must exist first */
147174
if(ref->isForward)
148175
return false;
149-
} else {
150-
if(!ref->isForward)
176+
} else if(!ref->isForward) {
177+
/* Inverse reference: only hierarchical refs create a
178+
* parent-child ordering dependency. */
179+
bool isHier = true; /* Conservative default */
180+
if(isHierCb)
181+
isHier = isHierCb(userCtx, &ref->refType);
182+
if(isHier)
151183
return false;
152184
}
153185
}
@@ -166,7 +198,7 @@ Nodeset_sortNodeClass(Nodeset *nodeset, NL_NodeClass nodeClass) {
166198
oldSize = nc->size;
167199
for(size_t i = 0; i < nc->size; i++) {
168200
NL_Node *node = nc->nodes[i];
169-
if(!nodeRefsReady(node))
201+
if(!nodeRefsReady(nodeset, node))
170202
continue;
171203
NodeContainer_add(&nodeset->sortedNodes, node);
172204
NodeContainer_remove(nc, i);

0 commit comments

Comments
 (0)