From 7e87fc11eb77b42141170b84e19425ed70554543 Mon Sep 17 00:00:00 2001 From: "Klaus Landsdorf (CEO)" Date: Sun, 27 Sep 2026 19:34:22 +0200 Subject: [PATCH 1/2] fix(types): fix ArrayOf.ToArrayOf(IEnumerable) overrun on a growing source The NET8_0_OR_GREATER fast path reads TryGetNonEnumeratedCount, then copies into a T[count] buffer. The count is a live Count read (e.g. a ConcurrentQueue a producer enqueues to), so it is not atomic with the foreach below: a source that grows after the count check indexes past the buffer with IndexOutOfRangeException, and one that shrinks returns an array padded with default(T) at the tail instead of its actual length. Found via a failing fork CI run of a managed-session reconnect test (ManagedSessionReconnectHarness.Requests snapshotting a ConcurrentQueue shared with the scripted transport). Grow the buffer instead of indexing past it, and trim it if the source enumerated shorter than its reported count. Reproduced deterministically with an ICollection whose Count does not match its enumeration (TryGetNonEnumeratedCount only takes the fast path for ICollection, not the read-only interface). --- src/Opc.Ua.Types/BuiltIn/ArrayOf.cs | 19 ++-- .../BuiltIn/ArrayOfTests.cs | 86 +++++++++++++++++++ 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/src/Opc.Ua.Types/BuiltIn/ArrayOf.cs b/src/Opc.Ua.Types/BuiltIn/ArrayOf.cs index 7f5f323a70..19b2773e9b 100644 --- a/src/Opc.Ua.Types/BuiltIn/ArrayOf.cs +++ b/src/Opc.Ua.Types/BuiltIn/ArrayOf.cs @@ -924,17 +924,26 @@ public static ArrayOf ToArrayOf(this IEnumerable? values) #if NET8_0_OR_GREATER if (values.TryGetNonEnumeratedCount(out int count)) { - if (count == 0) - { - return []; - } + // TryGetNonEnumeratedCount reads a live Count (e.g. a + // ConcurrentQueue shared with a producer), which is not + // atomic with the enumeration below: the source can grow or + // shrink before or while it is enumerated. Grow the buffer + // rather than index past it, and trim if it enumerated short. var copy = new T[count]; int index = 0; foreach (T item in values) { + if (index == copy.Length) + { + Array.Resize(ref copy, Math.Max(copy.Length * 2, 4)); + } copy[index++] = item; } - return new(copy); + if (index != copy.Length) + { + Array.Resize(ref copy, index); + } + return index == 0 ? [] : new(copy); } #endif #pragma warning disable RCS1151 // Cast explicit to avoid covariant conversion diff --git a/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs b/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs index 752001ab0d..fe43c0c4e1 100644 --- a/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs +++ b/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs @@ -543,6 +543,92 @@ public void ToArrayOfFromEmptyIEnumerableTest() Assert.That(arrayOf.IsEmpty, Is.True); } + [Test] + public void ToArrayOfFromIEnumerableThatGrowsPastItsReportedCountTest() + { + // TryGetNonEnumeratedCount reads a live Count that is not atomic + // with the enumeration, e.g. a ConcurrentQueue a producer + // enqueues to between the count check and the foreach below. + int[] items = [1, 2, 3, 4, 5]; + var enumerable = new StaleCountCollection(reportedCount: 2, items); + + var arrayOf = enumerable.ToArrayOf(); + + Assert.That(arrayOf.Count, Is.EqualTo(items.Length)); + Assert.That(arrayOf.Span.ToArray(), Is.EquivalentTo(items)); + } + + [Test] + public void ToArrayOfFromIEnumerableThatShrinksBelowItsReportedCountTest() + { + int[] items = [1, 2]; + var enumerable = new StaleCountCollection(reportedCount: 5, items); + + var arrayOf = enumerable.ToArrayOf(); + + Assert.That(arrayOf.Count, Is.EqualTo(items.Length)); + Assert.That(arrayOf.Span.ToArray(), Is.EquivalentTo(items)); + } + + /// + /// An whose does not + /// match the number of elements yields, + /// reproducing what a concurrent collection can report to + /// , + /// which only takes the fast path for + /// and not for the read-only interface. + /// + /// Type of the element in the collection. + private sealed class StaleCountCollection : ICollection + { + private readonly T[] m_items; + + public StaleCountCollection(int reportedCount, T[] items) + { + Count = reportedCount; + m_items = items; + } + + public int Count { get; } + + public bool IsReadOnly => true; + + public IEnumerator GetEnumerator() + { + return ((IEnumerable)m_items).GetEnumerator(); + } + + IEnumerator IEnumerable.GetEnumerator() + { + return GetEnumerator(); + } + + public void Add(T item) + { + throw new NotSupportedException(); + } + + public void Clear() + { + throw new NotSupportedException(); + } + + public bool Contains(T item) + { + throw new NotSupportedException(); + } + + public void CopyTo(T[] array, int arrayIndex) + { + throw new NotSupportedException(); + } + + public bool Remove(T item) + { + throw new NotSupportedException(); + } + } + [Test] public void ToArrayOfFromArrayTest() { From 10d6e4b22e5c19f4166d948534746fbe7b7db708 Mon Sep 17 00:00:00 2001 From: "Klaus Landsdorf (CEO)" Date: Sun, 27 Sep 2026 21:46:56 +0200 Subject: [PATCH 2/2] test(types): mirror ConcurrentQueue's actual interfaces in the repro double ConcurrentQueue implements the non-generic ICollection and IReadOnlyCollection, not the generic ICollection. The double implemented ICollection instead, which made the net48/net472 LINQ fallback (Enumerable.ToArray, used outside NET8_0_OR_GREATER) call its CopyTo(T[], int) and throw, failing the test for the wrong reason on those frameworks (confirmed on the fork CI's windows-net48 shard). --- .../BuiltIn/ArrayOfTests.cs | 43 ++++++------------- 1 file changed, 14 insertions(+), 29 deletions(-) diff --git a/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs b/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs index fe43c0c4e1..fdbe8321aa 100644 --- a/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs +++ b/tests/Opc.Ua.Types.Tests/BuiltIn/ArrayOfTests.cs @@ -571,15 +571,18 @@ public void ToArrayOfFromIEnumerableThatShrinksBelowItsReportedCountTest() } /// - /// An whose does not - /// match the number of elements yields, - /// reproducing what a concurrent collection can report to - /// , - /// which only takes the fast path for - /// and not for the read-only interface. + /// An whose does not match + /// the number of elements yields, mirroring + /// the interfaces + /// implements ( and , + /// not the generic ) so both the count-based + /// fast path of + /// + /// and the plain enumeration fallback on earlier target frameworks are + /// exercised the same way a live producer/consumer queue would. /// /// Type of the element in the collection. - private sealed class StaleCountCollection : ICollection + private sealed class StaleCountCollection : ICollection, IEnumerable, IReadOnlyCollection { private readonly T[] m_items; @@ -591,7 +594,9 @@ public StaleCountCollection(int reportedCount, T[] items) public int Count { get; } - public bool IsReadOnly => true; + public bool IsSynchronized => false; + + public object SyncRoot => this; public IEnumerator GetEnumerator() { @@ -603,27 +608,7 @@ IEnumerator IEnumerable.GetEnumerator() return GetEnumerator(); } - public void Add(T item) - { - throw new NotSupportedException(); - } - - public void Clear() - { - throw new NotSupportedException(); - } - - public bool Contains(T item) - { - throw new NotSupportedException(); - } - - public void CopyTo(T[] array, int arrayIndex) - { - throw new NotSupportedException(); - } - - public bool Remove(T item) + void ICollection.CopyTo(Array array, int index) { throw new NotSupportedException(); }