From 4bd1b9705fc3ad0f5fb05d314a434da2bf1eeea2 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Thu, 18 Apr 2019 21:36:10 -0400 Subject: [PATCH] Nullable: System.Collections.Concurrent (dotnet/coreclr#24054) Commit migrated from https://github.com/dotnet/coreclr/commit/c05d03ca6e4282df798e3f3dc81c05b427bd3fcb --- .../Collections/Concurrent/ConcurrentQueue.cs | 63 ++++++++++------------ .../Concurrent/ConcurrentQueueSegment.cs | 15 +++--- .../Concurrent/IProducerConsumerCollection.cs | 4 +- .../IProducerConsumerCollectionDebugView.cs | 8 +-- 4 files changed, 39 insertions(+), 51 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueue.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueue.cs index 17e7fd5..593e3a7 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueue.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueue.cs @@ -2,9 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable using System.Collections.Generic; using System.Diagnostics; -using System.Runtime.InteropServices; using System.Threading; namespace System.Collections.Concurrent @@ -52,7 +52,7 @@ namespace System.Collections.Concurrent /// Lock used to protect cross-segment operations, including any updates to or /// and any operations that need to get a consistent view of them. /// - private object _crossSegmentLock; + private readonly object _crossSegmentLock; /// The current tail segment. private volatile ConcurrentQueueSegment _tail; /// The current head segment. @@ -68,11 +68,20 @@ namespace System.Collections.Concurrent } /// - /// Initializes the contents of the queue from an existing collection. + /// Initializes a new instance of the class that contains elements copied + /// from the specified collection. /// - /// A collection from which to copy elements. - private void InitializeFromCollection(IEnumerable collection) + /// + /// The collection whose elements are copied to the new . + /// + /// The argument is null. + public ConcurrentQueue(IEnumerable collection) { + if (collection == null) + { + ThrowHelper.ThrowArgumentNullException(ExceptionArgument.collection); + } + _crossSegmentLock = new object(); // Determine the initial segment size. We'll use the default, @@ -91,31 +100,13 @@ namespace System.Collections.Concurrent // Initialize the segment and add all of the data to it. _tail = _head = new ConcurrentQueueSegment(length); - foreach (T item in collection) + foreach (T item in collection!) // TODO-NULLABLE: https://github.com/dotnet/csharplang/issues/538 { Enqueue(item); } } /// - /// Initializes a new instance of the class that contains elements copied - /// from the specified collection. - /// - /// - /// The collection whose elements are copied to the new . - /// - /// The argument is null. - public ConcurrentQueue(IEnumerable collection) - { - if (collection == null) - { - ThrowHelper.ThrowArgumentNullException(ExceptionArgument.collection); - } - - InitializeFromCollection(collection); - } - - /// /// Copies the elements of the to an , starting at a particular index. /// @@ -156,7 +147,7 @@ namespace System.Collections.Concurrent // Otherwise, fall back to the slower path that first copies the contents // to an array, and then uses that array's non-generic CopyTo to do the copy. - ToArray().CopyTo(array, index); + ToArray().CopyTo(array!, index); // TODO-NULLABLE: https://github.com/dotnet/csharplang/issues/538 } /// @@ -173,7 +164,7 @@ namespace System.Collections.Concurrent /// cref="ICollection"/>. This property is not supported. /// /// The SyncRoot property is not supported. - object ICollection.SyncRoot { get { ThrowHelper.ThrowNotSupportedException(ExceptionResource.ConcurrentCollection_SyncRoot_NotSupported); return default; } } + object ICollection.SyncRoot { get { ThrowHelper.ThrowNotSupportedException(ExceptionResource.ConcurrentCollection_SyncRoot_NotSupported); return default!; } } // TODO-NULLABLE: https://github.com/dotnet/csharplang/issues/538 /// Returns an enumerator that iterates through a collection. /// An that can be used to iterate through the collection. @@ -347,7 +338,7 @@ namespace System.Collections.Concurrent // With the cross-segment lock held, we're guaranteed that all of these internal segments are // consistent, as the head and tail segment can't be changed while we're holding the lock, and // dequeueing and enqueueing can only be done from the head and tail segments, which these aren't. - for (ConcurrentQueueSegment s = head._nextSegment; s != tail; s = s._nextSegment) + for (ConcurrentQueueSegment s = head._nextSegment!; s != tail; s = s._nextSegment!) { Debug.Assert(s._frozenForEnqueues, "Internal segment must be frozen as there's a following segment."); count += s._headAndTail.Tail - s.FreezeOffset; @@ -415,7 +406,7 @@ namespace System.Collections.Concurrent // Since there were segments before these, for our purposes we consider them to start at // the 0th element, and since there is at least one segment after each, each was frozen // by the time we snapped it, so we can iterate until each's frozen tail. - for (ConcurrentQueueSegment s = head._nextSegment; s != tail; s = s._nextSegment) + for (ConcurrentQueueSegment s = head._nextSegment!; s != tail; s = s._nextSegment!) { Debug.Assert(s._preservedForObservation); Debug.Assert(s._frozenForEnqueues); @@ -471,7 +462,7 @@ namespace System.Collections.Concurrent // Get the number of items to be enumerated long count = GetCount(head, headHead, tail, tailTail); - if (index > array.Length - count) + if (index > array!.Length - count) // TODO-NULLABLE: https://github.com/dotnet/csharplang/issues/538 { ThrowHelper.ThrowArgumentException(ExceptionResource.Arg_ArrayPlusOffTooSmall); } @@ -523,7 +514,7 @@ namespace System.Collections.Concurrent // Mark them and all segments in between as preserving, and ensure no additional items // can be added to the tail. - for (ConcurrentQueueSegment s = head; ; s = s._nextSegment) + for (ConcurrentQueueSegment s = head; ; s = s._nextSegment!) { s._preservedForObservation = true; if (s == tail) break; @@ -595,7 +586,7 @@ namespace System.Collections.Concurrent { // Each segment between head and tail, not including head and tail. Since there were // segments before these, for our purposes we consider it to start at the 0th element. - for (ConcurrentQueueSegment s = head._nextSegment; s != tail; s = s._nextSegment) + for (ConcurrentQueueSegment s = head._nextSegment!; s != tail; s = s._nextSegment!) { Debug.Assert(s._preservedForObservation, "Would have had to been preserved as a segment part of enumeration"); Debug.Assert(s._frozenForEnqueues, "Would have had to be frozen for enqueues as it's intermediate"); @@ -688,7 +679,7 @@ namespace System.Collections.Concurrent /// true if an element was removed and returned from the beginning of the /// successfully; otherwise, false. /// - public bool TryDequeue(out T result) => + public bool TryDequeue(out T result) => // TODO-GENERIC-NULLABLE _head.TryDequeue(out result) || // fast-path that operates just on the head segment TryDequeueSlow(out result); // slow path that needs to fix up segments @@ -711,7 +702,7 @@ namespace System.Collections.Concurrent // check and this check, another item could have arrived). if (head._nextSegment == null) { - item = default; + item = default!; // TODO-NULLABLE-GENERIC return false; } @@ -752,7 +743,7 @@ namespace System.Collections.Concurrent /// For determining whether the collection contains any items, use of the /// property is recommended rather than peeking. /// - public bool TryPeek(out T result) => TryPeek(out result, resultUsed: true); + public bool TryPeek(out T result) => TryPeek(out result, resultUsed: true); // TODO-GENERIC-NULLABLE /// Attempts to retrieve the value for the first element in the queue. /// The value of the first element, if found. @@ -768,7 +759,7 @@ namespace System.Collections.Concurrent // Grab the next segment from this one, before we peek. // This is to be able to see whether the value has changed // during the peek operation. - ConcurrentQueueSegment next = Volatile.Read(ref s._nextSegment); + ConcurrentQueueSegment? next = Volatile.Read(ref s._nextSegment); // Peek at the segment. If we find an element, we're done. if (s.TryPeek(out result, resultUsed)) @@ -805,7 +796,7 @@ namespace System.Collections.Concurrent // and we'll traverse to that segment. } - result = default; + result = default!; // TODO-NULLABLE-GENERIC return false; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs index c724285..c120d77 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/ConcurrentQueueSegment.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable using System.Diagnostics; using System.Runtime.InteropServices; using System.Threading; @@ -33,7 +34,7 @@ namespace System.Collections.Concurrent internal bool _frozenForEnqueues; #pragma warning disable 0649 // some builds don't assign to this field /// The segment following this one in the queue, or null if this segment is the last in the queue. - internal ConcurrentQueueSegment _nextSegment; // SOS's ThreadPool command depends on this name + internal ConcurrentQueueSegment? _nextSegment; // SOS's ThreadPool command depends on this name #pragma warning restore 0649 /// Creates the segment. @@ -126,7 +127,7 @@ namespace System.Collections.Concurrent } /// Tries to dequeue an element from the queue. - public bool TryDequeue(out T item) + public bool TryDequeue(out T item) // TODO-NULLABLE-GENERIC { Slot[] slots = _slots; @@ -164,7 +165,7 @@ namespace System.Collections.Concurrent // If we're preserving, though, we don't zero out the slot, as we need it for // enumerations, peeking, ToArray, etc. And we don't update the sequence number, // so that an enqueuer will see it as full and be forced to move to a new segment. - slots[slotsIndex].Item = default(T); + slots[slotsIndex].Item = default!; // TODO-NULLABLE-GENERIC Volatile.Write(ref slots[slotsIndex].SequenceNumber, currentHead + slots.Length); } return true; @@ -183,7 +184,7 @@ namespace System.Collections.Concurrent int currentTail = Volatile.Read(ref _headAndTail.Tail); if (currentTail - currentHead <= 0 || (frozen && (currentTail - FreezeOffset - currentHead <= 0))) { - item = default(T); + item = default!; // TODO-NULLABLE-GENERIC return false; } @@ -198,7 +199,7 @@ namespace System.Collections.Concurrent } /// Tries to peek at an element from the queue, without removing it. - public bool TryPeek(out T result, bool resultUsed) + public bool TryPeek(out T result, bool resultUsed) // TODO-NULLABLE-GENERIC { if (resultUsed) { @@ -228,7 +229,7 @@ namespace System.Collections.Concurrent int diff = sequenceNumber - (currentHead + 1); if (diff == 0) { - result = resultUsed ? slots[slotsIndex].Item : default(T); + result = resultUsed ? slots[slotsIndex].Item : default!; // TODO-NULLABLE-GENERIC return true; } else if (diff < 0) @@ -244,7 +245,7 @@ namespace System.Collections.Concurrent int currentTail = Volatile.Read(ref _headAndTail.Tail); if (currentTail - currentHead <= 0 || (frozen && (currentTail - FreezeOffset - currentHead <= 0))) { - result = default(T); + result = default!; // TODO-NULLABLE-GENERIC return false; } diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollection.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollection.cs index 9727150..797bb08 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollection.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollection.cs @@ -2,8 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable using System.Collections.Generic; -using System.Diagnostics; namespace System.Collections.Concurrent { @@ -59,7 +59,7 @@ namespace System.Collections.Concurrent /// unspecified. /// /// true if an object was removed and returned successfully; otherwise, false. - bool TryTake(out T item); + bool TryTake(out T item); // TODO-NULLABLE-GENERIC /// /// Copies the elements contained in the to a new array. diff --git a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollectionDebugView.cs b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollectionDebugView.cs index 0205e06..f30f8f6 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollectionDebugView.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Collections/Concurrent/IProducerConsumerCollectionDebugView.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +#nullable enable using System.Diagnostics; namespace System.Collections.Concurrent @@ -21,12 +22,7 @@ namespace System.Collections.Concurrent /// A collection to browse in the debugger. public IProducerConsumerCollectionDebugView(IProducerConsumerCollection collection) { - if (collection == null) - { - throw new ArgumentNullException(nameof(collection)); - } - - _collection = collection; + _collection = collection ?? throw new ArgumentNullException(nameof(collection)); } /// -- 2.7.4