Skip to content

Commit dc661ba

Browse files
mregenmregen
authored
Fix server CurrentTime source timestamp and improve readerwriter locks (#2903)
Fix server time source timestamp is not updated. There are duplicate objects in generated code, on of which which holds the last timestamp was not updated. Move all readerwriter locks outside of try/finally to avoid a SynchronizationLockException when a reader writer lock is disposed or is cancelled. Currently there is no obvious fundamental bug in how the readerwriter locks are used, but outside the try/finally clause e.g. a disposed lock may not trigger a false exception.
1 parent f431d53 commit dc661ba

File tree

5 files changed

+68
-83
lines changed

5 files changed

+68
-83
lines changed

‎Libraries/Opc.Ua.Client/NodeCache/NodeCache.cs‎

Lines changed: 41 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,10 @@ public INode Find(ExpandedNodeId nodeId)
104104
}
105105

106106
INode node;
107+
108+
m_cacheLock.EnterReadLock();
107109
try
108110
{
109-
m_cacheLock.EnterReadLock();
110-
111111
// check if node already exists.
112112
node = m_nodes.Find(nodeId);
113113
}
@@ -155,10 +155,10 @@ public IList<INode> Find(IList<ExpandedNodeId> nodeIds)
155155
for (ii = 0; ii < count; ii++)
156156
{
157157
INode node;
158+
159+
m_cacheLock.EnterReadLock();
158160
try
159161
{
160-
m_cacheLock.EnterReadLock();
161-
162162
// check if node already exists.
163163
node = m_nodes.Find(nodeIds[ii]);
164164
}
@@ -235,10 +235,10 @@ public INode Find(
235235
}
236236

237237
IList<IReference> references;
238+
239+
m_cacheLock.EnterReadLock();
238240
try
239241
{
240-
m_cacheLock.EnterReadLock();
241-
242242
// find all references.
243243
references = source.ReferenceTable.Find(referenceTypeId, isInverse, includeSubtypes, m_typeTree);
244244
}
@@ -285,10 +285,9 @@ public IList<INode> Find(
285285
}
286286

287287
IList<IReference> references;
288+
m_cacheLock.EnterReadLock();
288289
try
289290
{
290-
m_cacheLock.EnterReadLock();
291-
292291
// find all references.
293292
references = source.ReferenceTable.Find(referenceTypeId, isInverse, includeSubtypes, m_typeTree);
294293
}
@@ -325,10 +324,9 @@ public bool IsKnown(ExpandedNodeId typeId)
325324
return false;
326325
}
327326

327+
m_cacheLock.EnterReadLock();
328328
try
329329
{
330-
m_cacheLock.EnterReadLock();
331-
332330
return m_typeTree.IsKnown(typeId);
333331
}
334332
finally
@@ -347,10 +345,9 @@ public bool IsKnown(NodeId typeId)
347345
return false;
348346
}
349347

348+
m_cacheLock.EnterReadLock();
350349
try
351350
{
352-
m_cacheLock.EnterReadLock();
353-
354351
return m_typeTree.IsKnown(typeId);
355352
}
356353
finally
@@ -369,10 +366,9 @@ public NodeId FindSuperType(ExpandedNodeId typeId)
369366
return null;
370367
}
371368

369+
m_cacheLock.EnterReadLock();
372370
try
373371
{
374-
m_cacheLock.EnterReadLock();
375-
376372
return m_typeTree.FindSuperType(typeId);
377373
}
378374
finally
@@ -392,10 +388,9 @@ public NodeId FindSuperType(NodeId typeId)
392388
return null;
393389
}
394390

391+
m_cacheLock.EnterReadLock();
395392
try
396393
{
397-
m_cacheLock.EnterReadLock();
398-
399394
return m_typeTree.FindSuperType(typeId);
400395
}
401396
finally
@@ -409,18 +404,18 @@ public NodeId FindSuperType(NodeId typeId)
409404
public IList<NodeId> FindSubTypes(ExpandedNodeId typeId)
410405
{
411406
ILocalNode type = Find(typeId) as ILocalNode;
407+
List<NodeId> subtypes = new List<NodeId>();
412408

413409
if (type == null)
414410
{
415-
return new List<NodeId>();
411+
return subtypes;
416412
}
417413

418-
List<NodeId> subtypes = new List<NodeId>();
419414
IList<IReference> references;
415+
416+
m_cacheLock.EnterReadLock();
420417
try
421418
{
422-
m_cacheLock.EnterReadLock();
423-
424419
references = type.References.Find(ReferenceTypeIds.HasSubtype, false, true, m_typeTree);
425420
}
426421
finally
@@ -459,10 +454,10 @@ public bool IsTypeOf(ExpandedNodeId subTypeId, ExpandedNodeId superTypeId)
459454
while (supertype != null)
460455
{
461456
ExpandedNodeId currentId;
457+
458+
m_cacheLock.EnterReadLock();
462459
try
463460
{
464-
m_cacheLock.EnterReadLock();
465-
466461
currentId = supertype.References.FindTarget(ReferenceTypeIds.HasSubtype, true, true, m_typeTree, 0);
467462
}
468463
finally
@@ -501,10 +496,10 @@ public bool IsTypeOf(NodeId subTypeId, NodeId superTypeId)
501496
while (supertype != null)
502497
{
503498
ExpandedNodeId currentId;
499+
500+
m_cacheLock.EnterReadLock();
504501
try
505502
{
506-
m_cacheLock.EnterReadLock();
507-
508503
currentId = supertype.References.FindTarget(ReferenceTypeIds.HasSubtype, true, true, m_typeTree, 0);
509504
}
510505
finally
@@ -526,10 +521,9 @@ public bool IsTypeOf(NodeId subTypeId, NodeId superTypeId)
526521
/// <inheritdoc/>
527522
public QualifiedName FindReferenceTypeName(NodeId referenceTypeId)
528523
{
524+
m_cacheLock.EnterReadLock();
529525
try
530526
{
531-
m_cacheLock.EnterReadLock();
532-
533527
return m_typeTree.FindReferenceTypeName(referenceTypeId);
534528
}
535529
finally
@@ -541,10 +535,9 @@ public QualifiedName FindReferenceTypeName(NodeId referenceTypeId)
541535
/// <inheritdoc/>
542536
public NodeId FindReferenceType(QualifiedName browseName)
543537
{
538+
m_cacheLock.EnterReadLock();
544539
try
545540
{
546-
m_cacheLock.EnterReadLock();
547-
548541
return m_typeTree.FindReferenceType(browseName);
549542
}
550543
finally
@@ -564,10 +557,10 @@ public bool IsEncodingOf(ExpandedNodeId encodingId, ExpandedNodeId datatypeId)
564557
}
565558

566559
IList<IReference> references;
560+
561+
m_cacheLock.EnterReadLock();
567562
try
568563
{
569-
m_cacheLock.EnterReadLock();
570-
571564
references = encoding.References.Find(ReferenceTypeIds.HasEncoding, true, true, m_typeTree);
572565
}
573566
finally
@@ -611,10 +604,10 @@ public bool IsEncodingFor(NodeId expectedTypeId, ExtensionObject value)
611604
}
612605

613606
IList<IReference> references;
607+
608+
m_cacheLock.EnterReadLock();
614609
try
615610
{
616-
m_cacheLock.EnterReadLock();
617-
618611
references = encoding.References.Find(ReferenceTypeIds.HasEncoding, true, true, m_typeTree);
619612
}
620613
finally
@@ -706,10 +699,10 @@ public NodeId FindDataTypeId(ExpandedNodeId encodingId)
706699
}
707700

708701
IList<IReference> references;
702+
703+
m_cacheLock.EnterReadLock();
709704
try
710705
{
711-
m_cacheLock.EnterReadLock();
712-
713706
references = encoding.References.Find(ReferenceTypeIds.HasEncoding, true, true, m_typeTree);
714707
}
715708
finally
@@ -736,10 +729,10 @@ public NodeId FindDataTypeId(NodeId encodingId)
736729
}
737730

738731
IList<IReference> references;
732+
733+
m_cacheLock.EnterReadLock();
739734
try
740735
{
741-
m_cacheLock.EnterReadLock();
742-
743736
references = encoding.References.Find(ReferenceTypeIds.HasEncoding, true, true, m_typeTree);
744737
}
745738
finally
@@ -769,10 +762,9 @@ public void LoadUaDefinedTypes(ISystemContext context)
769762
var assembly = typeof(ArgumentCollection).GetTypeInfo().Assembly;
770763
predefinedNodes.LoadFromBinaryResource(context, "Opc.Ua.Stack.Generated.Opc.Ua.PredefinedNodes.uanodes", assembly, true);
771764

765+
m_cacheLock.EnterWriteLock();
772766
try
773767
{
774-
m_cacheLock.EnterWriteLock();
775-
776768
for (int ii = 0; ii < predefinedNodes.Count; ii++)
777769
{
778770
BaseTypeState type = predefinedNodes[ii] as BaseTypeState;
@@ -795,11 +787,10 @@ public void LoadUaDefinedTypes(ISystemContext context)
795787
/// <inheritdoc/>
796788
public void Clear()
797789
{
798-
m_uaTypesLoaded = false;
790+
m_cacheLock.EnterWriteLock();
799791
try
800792
{
801-
m_cacheLock.EnterWriteLock();
802-
793+
m_uaTypesLoaded = false;
803794
m_nodes.Clear();
804795
}
805796
finally
@@ -826,10 +817,9 @@ public Node FetchNode(ExpandedNodeId nodeId)
826817
// fetch references from server.
827818
ReferenceDescriptionCollection references = m_session.FetchReferences(localId);
828819

820+
m_cacheLock.EnterUpgradeableReadLock();
829821
try
830822
{
831-
m_cacheLock.EnterUpgradeableReadLock();
832-
833823
foreach (ReferenceDescription reference in references)
834824
{
835825
// create a placeholder for the node if it does not already exist.
@@ -896,10 +886,9 @@ public IList<Node> FetchNodes(IList<ExpandedNodeId> nodeIds)
896886

897887
foreach (ReferenceDescription reference in references)
898888
{
889+
m_cacheLock.EnterUpgradeableReadLock();
899890
try
900891
{
901-
m_cacheLock.EnterUpgradeableReadLock();
902-
903892
// create a placeholder for the node if it does not already exist.
904893
if (!m_nodes.Exists(reference.NodeId))
905894
{
@@ -976,10 +965,10 @@ public IList<INode> FindReferences(
976965
}
977966

978967
IList<IReference> references;
968+
969+
m_cacheLock.EnterReadLock();
979970
try
980971
{
981-
m_cacheLock.EnterReadLock();
982-
983972
references = source.ReferenceTable.Find(referenceTypeId, isInverse, includeSubtypes, m_typeTree);
984973
}
985974
finally
@@ -1026,10 +1015,10 @@ public IList<INode> FindReferences(
10261015
foreach (var referenceTypeId in referenceTypeIds)
10271016
{
10281017
IList<IReference> references;
1018+
1019+
m_cacheLock.EnterReadLock();
10291020
try
10301021
{
1031-
m_cacheLock.EnterReadLock();
1032-
10331022
references = node.ReferenceTable.Find(referenceTypeId, isInverse, includeSubtypes, m_typeTree);
10341023
}
10351024
finally
@@ -1077,10 +1066,10 @@ public string GetDisplayText(INode node)
10771066
NodeId modellingRule = target.ModellingRule;
10781067

10791068
IList<IReference> references;
1069+
1070+
m_cacheLock.EnterReadLock();
10801071
try
10811072
{
1082-
m_cacheLock.EnterReadLock();
1083-
10841073
references = target.ReferenceTable.Find(ReferenceTypeIds.Aggregates, true, true, m_typeTree);
10851074
}
10861075
finally
@@ -1167,10 +1156,9 @@ public NodeId BuildBrowsePath(ILocalNode node, IList<QualifiedName> browsePath)
11671156
#region Private Methods
11681157
private void InternalWriteLockedAttach(ILocalNode node)
11691158
{
1159+
m_cacheLock.EnterWriteLock();
11701160
try
11711161
{
1172-
m_cacheLock.EnterWriteLock();
1173-
11741162
// add to cache.
11751163
m_nodes.Attach(node);
11761164
}

0 commit comments

Comments
 (0)