From 3b6379c387ac1c3a4361045590afa457ee68c772 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Mon, 8 Jul 2024 10:58:44 -0400 Subject: [PATCH] Use generic Interlocked.{Compare}Exchange in more places --- .../src/System/EventReporter.cs | 4 +- .../Common/src/System/Net/StreamBuffer.cs | 12 +-- ...0108HmacCounterKdfImplementationManaged.cs | 4 +- .../System/Net/WebSockets/WebSocketStream.cs | 8 +- .../Collections/Concurrent/ConcurrentBag.cs | 26 +++--- .../tests/TypeDescriptorTests.cs | 6 +- .../src/System/IO/Compression/BrotliStream.cs | 15 ++-- .../Compression/DeflateZLib/DeflateStream.cs | 21 +++-- .../Parallel/Channels/AsynchronousChannel.cs | 24 ++--- .../SocketsHttpHandler/Http3Connection.cs | 14 +-- .../Http/SocketsHttpHandler/HttpConnection.cs | 7 +- .../HttpContentReadStream.cs | 10 +-- .../Net/Windows/HttpListener.Windows.cs | 28 +++--- .../Net/Windows/WebSockets/WebSocketBase.cs | 23 +++-- .../Net/Windows/WebSockets/WebSocketBuffer.cs | 12 +-- .../WebSocketHttpListenerDuplexStream.cs | 24 ++--- .../src/System/Net/Mail/SmtpClient.cs | 6 +- .../src/System/Net/Quic/QuicConnection.cs | 16 ++-- .../src/System/Net/Quic/QuicListener.cs | 8 +- .../src/System/Net/Quic/QuicStream.Stream.cs | 16 ++-- .../src/System/Net/Quic/QuicStream.cs | 14 +-- .../src/System/Net/HttpWebRequest.cs | 22 ++--- .../src/System/Net/TimerThread.cs | 4 +- .../System/Net/Security/NegotiateStream.cs | 18 ++-- .../src/System/Net/Security/SslStream.IO.cs | 58 ++++++------ .../src/System/Net/Security/SslStream.cs | 18 ++-- .../Fakes/FakeSslStream.Implementation.cs | 4 +- .../src/System/Net/Sockets/NetworkStream.cs | 8 +- .../System/Net/Sockets/SafeSocketHandle.cs | 5 +- .../src/System/Net/Sockets/Socket.Unix.cs | 2 +- .../src/System/Net/Sockets/Socket.cs | 6 +- .../Net/Sockets/SocketAsyncContext.Unix.cs | 42 ++++----- .../Net/Sockets/SocketAsyncEngine.Unix.cs | 30 +++---- .../Sockets/SocketAsyncEventArgs.Windows.cs | 4 +- .../Net/Sockets/SocketAsyncEventArgs.cs | 38 ++++---- .../src/System/Net/Sockets/TCPClient.cs | 6 +- .../src/System/Net/WebProxy.NonBrowser.cs | 6 +- .../System/Net/WebSockets/ClientWebSocket.cs | 22 ++--- .../src/System/Buffers/SharedArrayPool.cs | 4 +- .../Diagnostics/Tracing/ActivityTracker.cs | 15 ++-- .../Threading/CancellationTokenSource.cs | 49 +++++----- .../System/Threading/ReaderWriterLockSlim.cs | 12 +-- .../System/Threading/ThreadPoolWorkQueue.cs | 89 +++++++++---------- .../WaitSubsystem.ThreadWaitInfo.Unix.cs | 10 +-- .../InteropServices/Marshalling/ComObject.cs | 7 +- .../Threading/Tasks/Parallel.ForEachAsync.cs | 44 +++------ .../Threading/Tasks/ParallelLoopState.cs | 4 +- .../Threading/Tasks/ParallelRangeManager.cs | 10 +-- .../tests/ThreadPoolTests.cs | 6 +- 49 files changed, 417 insertions(+), 424 deletions(-) diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EventReporter.cs b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EventReporter.cs index efb46025dbf5b5..1fc9df30b932ad 100644 --- a/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EventReporter.cs +++ b/src/coreclr/nativeaot/System.Private.CoreLib/src/System/EventReporter.cs @@ -171,7 +171,7 @@ private static unsafe void ClrReportEvent(string eventSource, short type, ushort Interop.Advapi32.DeregisterEventSource(handle); } - private static byte s_once; + private static bool s_once; public static bool ShouldLogInEventLog { @@ -180,7 +180,7 @@ public static bool ShouldLogInEventLog if (Interop.Kernel32.IsDebuggerPresent()) return false; - if (s_once == 1 || Interlocked.Exchange(ref s_once, 1) == 1) + if (s_once || Interlocked.Exchange(ref s_once, true)) return false; return true; diff --git a/src/libraries/Common/src/System/Net/StreamBuffer.cs b/src/libraries/Common/src/System/Net/StreamBuffer.cs index 32bc0f3f4e45ce..4af8e4bd75171b 100644 --- a/src/libraries/Common/src/System/Net/StreamBuffer.cs +++ b/src/libraries/Common/src/System/Net/StreamBuffer.cs @@ -292,7 +292,7 @@ private sealed class ResettableValueTaskSource : IValueTaskSource private ManualResetValueTaskSourceCore _waitSource; // mutable struct, do not make this readonly private CancellationTokenRegistration _waitSourceCancellation; - private int _hasWaiter; + private bool _hasWaiter; ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => _waitSource.GetStatus(token); @@ -300,7 +300,7 @@ private sealed class ResettableValueTaskSource : IValueTaskSource void IValueTaskSource.GetResult(short token) { - Debug.Assert(_hasWaiter == 0); + Debug.Assert(!_hasWaiter); // Clean up the registration. This will wait for any in-flight cancellation to complete. _waitSourceCancellation.Dispose(); @@ -312,7 +312,7 @@ void IValueTaskSource.GetResult(short token) public void SignalWaiter() { - if (Interlocked.Exchange(ref _hasWaiter, 0) == 1) + if (Interlocked.Exchange(ref _hasWaiter, false)) { _waitSource.SetResult(true); } @@ -322,7 +322,7 @@ private void CancelWaiter(CancellationToken cancellationToken) { Debug.Assert(cancellationToken.IsCancellationRequested); - if (Interlocked.Exchange(ref _hasWaiter, 0) == 1) + if (Interlocked.Exchange(ref _hasWaiter, false)) { _waitSource.SetException(ExceptionDispatchInfo.SetCurrentStackTrace(new OperationCanceledException(cancellationToken))); } @@ -330,13 +330,13 @@ private void CancelWaiter(CancellationToken cancellationToken) public void Reset() { - if (_hasWaiter != 0) + if (_hasWaiter) { throw new InvalidOperationException("Concurrent use is not supported"); } _waitSource.Reset(); - Volatile.Write(ref _hasWaiter, 1); + Volatile.Write(ref _hasWaiter, true); } public void Wait() diff --git a/src/libraries/Common/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationManaged.cs b/src/libraries/Common/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationManaged.cs index 60c64aa520e4be..3e027d68dab28b 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationManaged.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/SP800108HmacCounterKdfImplementationManaged.cs @@ -14,7 +14,7 @@ internal sealed partial class SP800108HmacCounterKdfImplementationManaged : SP80 { private byte[] _key; private int _keyReferenceCount = 1; - private int _disposed; + private bool _disposed; private readonly HashAlgorithmName _hashAlgorithm; internal override void DeriveBytes(ReadOnlySpan label, ReadOnlySpan context, Span destination) @@ -61,7 +61,7 @@ internal override void DeriveBytes(byte[] label, byte[] context, Span dest public override void Dispose() { - if (Interlocked.Exchange(ref _disposed, 1) == 0) + if (!Interlocked.Exchange(ref _disposed, true)) { ReleaseKey(); } diff --git a/src/libraries/Common/tests/System/Net/WebSockets/WebSocketStream.cs b/src/libraries/Common/tests/System/Net/WebSockets/WebSocketStream.cs index f025a7977f28b8..5666195ac8628e 100644 --- a/src/libraries/Common/tests/System/Net/WebSockets/WebSocketStream.cs +++ b/src/libraries/Common/tests/System/Net/WebSockets/WebSocketStream.cs @@ -21,8 +21,8 @@ public class WebSocketStream : Stream // Used by the class to indicate that the stream is writable. private bool _writeable; - // Whether Dispose has been called. 0 == false, 1 == true - private int _disposed; + // Whether Dispose has been called. + private bool _disposed; public WebSocketStream(WebSocket socket) : this(socket, FileAccess.ReadWrite, ownsSocket: false) @@ -140,7 +140,7 @@ public void Close(int timeout) protected override void Dispose(bool disposing) { - if (Interlocked.Exchange(ref _disposed, 1) != 0) + if (Interlocked.Exchange(ref _disposed, true)) { return; } @@ -269,7 +269,7 @@ public override void SetLength(long value) private void ThrowIfDisposed() { - ObjectDisposedException.ThrowIf(_disposed != 0, this); + ObjectDisposedException.ThrowIf(_disposed, this); } private static IOException WrapException(string resourceFormatString, Exception innerException) diff --git a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs index 544aa251d1be67..acc0f1f4d8445c 100644 --- a/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs +++ b/src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs @@ -628,11 +628,11 @@ private void FreezeBag(ref bool lockTaken) // Finally, wait for all unsynchronized operations on each queue to be done. for (WorkStealingQueue? queue = head; queue != null; queue = queue._nextQueue) { - if (queue._currentOp != (int)Operation.None) + if (queue._currentOp != Operation.None) { SpinWait spinner = default; do { spinner.SpinOnce(); } - while (queue._currentOp != (int)Operation.None); + while (queue._currentOp != Operation.None); } } } @@ -684,7 +684,7 @@ private sealed class WorkStealingQueue /// Number of steals; needs to be combined with to get an actual Count. private int _stealCount; /// The current queue operation. Used to quiesce before performing operations from one thread onto another. - internal volatile int _currentOp; + internal volatile Operation _currentOp; /// true if this queue's lock is held as part of a global freeze. internal bool _frozen; /// Next queue in the 's set of thread-local queues. @@ -726,14 +726,14 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount) try { // Full fence to ensure subsequent reads don't get reordered before this - Interlocked.Exchange(ref _currentOp, (int)Operation.Add); + Interlocked.Exchange(ref _currentOp, Operation.Add); int tail = _tailIndex; // Rare corner case (at most once every 2 billion pushes on this thread): // We're going to increment the tail; if we'll overflow, then we need to reset our counts if (tail == int.MaxValue) { - _currentOp = (int)Operation.None; // set back to None temporarily to avoid a deadlock + _currentOp = Operation.None; // set back to None temporarily to avoid a deadlock lock (this) { Debug.Assert(_tailIndex == tail, "No other thread should be changing _tailIndex"); @@ -749,7 +749,7 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount) _tailIndex = tail &= _mask; Debug.Assert(_headIndex - _tailIndex <= 0); - Interlocked.Exchange(ref _currentOp, (int)Operation.Add); // ensure subsequent reads aren't reordered before this + Interlocked.Exchange(ref _currentOp, Operation.Add); // ensure subsequent reads aren't reordered before this } } @@ -778,7 +778,7 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount) else { // We need to contend with foreign operations (e.g. steals, enumeration, etc.), so we lock. - _currentOp = (int)Operation.None; // set back to None to avoid a deadlock + _currentOp = Operation.None; // set back to None to avoid a deadlock Monitor.Enter(this, ref lockTaken); head = _headIndex; @@ -830,7 +830,7 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount) } finally { - _currentOp = (int)Operation.None; + _currentOp = Operation.None; if (lockTaken) { Monitor.Exit(this); @@ -874,7 +874,7 @@ internal bool TryLocalPop([MaybeNullWhen(false)] out T result) // If the read of _headIndex moved before this write to _tailIndex, we could erroneously end up // popping an element that's concurrently being stolen, leading to the same element being // dequeued from the bag twice. - _currentOp = (int)Operation.Take; + _currentOp = Operation.Take; Interlocked.Exchange(ref _tailIndex, --tail); // If there is no interaction with a steal, we can head down the fast path. @@ -895,7 +895,7 @@ internal bool TryLocalPop([MaybeNullWhen(false)] out T result) else { // Interaction with steals: 0 or 1 elements left. - _currentOp = (int)Operation.None; // set back to None to avoid a deadlock + _currentOp = Operation.None; // set back to None to avoid a deadlock Monitor.Enter(this, ref lockTaken); if (_headIndex - tail <= 0) { @@ -920,7 +920,7 @@ internal bool TryLocalPop([MaybeNullWhen(false)] out T result) } finally { - _currentOp = (int)Operation.None; + _currentOp = Operation.None; if (lockTaken) { Monitor.Exit(this); @@ -975,14 +975,14 @@ internal bool TrySteal([MaybeNullWhen(false)] out T result, bool take) // is in progress, as add operations need to accurately count transitions // from empty to non-empty, and they can only do that if there are no concurrent // steal operations happening at the time. - if ((head - (_tailIndex - 2) >= 0) && _currentOp == (int)Operation.Add) + if ((head - (_tailIndex - 2) >= 0) && _currentOp == Operation.Add) { SpinWait spinner = default; do { spinner.SpinOnce(); } - while (_currentOp == (int)Operation.Add); + while (_currentOp == Operation.Add); } // Increment head to tentatively take an element: a full fence is used to ensure the read diff --git a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs index a9b6a2cd70919d..6fb9f57edc479f 100644 --- a/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs +++ b/src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs @@ -1276,11 +1276,11 @@ class TwiceDerivedCultureInfo : DerivedCultureInfo { } - private long _concurrentError = 0; + private volatile bool _concurrentError; private bool ConcurrentError { - get => Interlocked.Read(ref _concurrentError) == 1; - set => Interlocked.Exchange(ref _concurrentError, value ? 1 : 0); + get => _concurrentError; + set => Interlocked.Exchange(ref _concurrentError, value); } private void ConcurrentTest(TypeWithProperty instance) diff --git a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs index a7a9e603f1a5b1..002186c9b36511 100644 --- a/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs +++ b/src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs @@ -128,7 +128,7 @@ private void ReleaseStateForDispose() if (buffer != null) { _buffer = null!; - if (!AsyncOperationIsActive) + if (!_activeAsyncOperation) { ArrayPool.Shared.Return(buffer); } @@ -170,18 +170,19 @@ public override long Position /// The length of the stream. public override void SetLength(long value) => throw new NotSupportedException(); - private int _activeAsyncOperation; // 1 == true, 0 == false - private bool AsyncOperationIsActive => _activeAsyncOperation != 0; + private volatile bool _activeAsyncOperation; private void EnsureNoActiveAsyncOperation() { - if (AsyncOperationIsActive) + if (_activeAsyncOperation) + { ThrowInvalidBeginCall(); + } } private void AsyncOperationStarting() { - if (Interlocked.Exchange(ref _activeAsyncOperation, 1) != 0) + if (Interlocked.Exchange(ref _activeAsyncOperation, true)) { ThrowInvalidBeginCall(); } @@ -189,8 +190,8 @@ private void AsyncOperationStarting() private void AsyncOperationCompleting() { - Debug.Assert(_activeAsyncOperation == 1); - Volatile.Write(ref _activeAsyncOperation, 0); + Debug.Assert(_activeAsyncOperation); + _activeAsyncOperation = false; } private static void ThrowInvalidBeginCall() => diff --git a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs index 07af0f3a93efbc..2ae2804ef6f9a1 100644 --- a/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs +++ b/src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs @@ -20,7 +20,7 @@ public partial class DeflateStream : Stream private Inflater? _inflater; private Deflater? _deflater; private byte[]? _buffer; - private int _activeAsyncOperation; // 1 == true, 0 == false + private volatile bool _activeAsyncOperation; private bool _wroteBytes; internal DeflateStream(Stream stream, CompressionMode mode, long uncompressedSize) : this(stream, mode, leaveOpen: false, ZLibNative.Deflate_DefaultWindowBits, uncompressedSize) @@ -698,7 +698,7 @@ protected override void Dispose(bool disposing) if (buffer != null) { _buffer = null; - if (!AsyncOperationIsActive) + if (!_activeAsyncOperation) { ArrayPool.Shared.Return(buffer); } @@ -751,7 +751,7 @@ async ValueTask Core() if (buffer != null) { _buffer = null; - if (!AsyncOperationIsActive) + if (!_activeAsyncOperation) { ArrayPool.Shared.Return(buffer); } @@ -1059,24 +1059,27 @@ public override void Flush() { } public override void SetLength(long value) { throw new NotSupportedException(); } } - private bool AsyncOperationIsActive => _activeAsyncOperation != 0; - private void EnsureNoActiveAsyncOperation() { - if (AsyncOperationIsActive) + if (_activeAsyncOperation) + { ThrowInvalidBeginCall(); + } } private void AsyncOperationStarting() { - if (Interlocked.Exchange(ref _activeAsyncOperation, 1) != 0) + if (Interlocked.Exchange(ref _activeAsyncOperation, true)) { ThrowInvalidBeginCall(); } } - private void AsyncOperationCompleting() => - Volatile.Write(ref _activeAsyncOperation, 0); + private void AsyncOperationCompleting() + { + Debug.Assert(_activeAsyncOperation); + _activeAsyncOperation = false; + } private static void ThrowInvalidBeginCall() => throw new InvalidOperationException(SR.InvalidBeginCall); diff --git a/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Channels/AsynchronousChannel.cs b/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Channels/AsynchronousChannel.cs index 775228fb66c5f0..6bffc272ee8e2d 100644 --- a/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Channels/AsynchronousChannel.cs +++ b/src/libraries/System.Linq.Parallel/src/System/Linq/Parallel/Channels/AsynchronousChannel.cs @@ -86,10 +86,10 @@ internal sealed class AsynchronousChannel : IDisposable private ManualResetEventSlim? _producerEvent; private IntValueEvent? _consumerEvent; - // These two-valued ints track whether a producer or consumer _might_ be waiting. They are marked + // These bools track whether a producer or consumer _might_ be waiting. They are marked // volatile because they are used in synchronization critical regions of code (see usage below). - private volatile int _producerIsWaiting; - private volatile int _consumerIsWaiting; + private volatile bool _producerIsWaiting; + private volatile bool _consumerIsWaiting; private readonly CancellationToken _cancellationToken; //----------------------------------------------------------------------------------- @@ -309,11 +309,11 @@ private void EnqueueChunk(T[] chunk) // our producer index doesn't pass the read of the consumer waiting flags; the CLR memory // model unfortunately permits this reordering. That is handled by using a CAS above.) - if (_consumerIsWaiting == 1 && !IsChunkBufferEmpty) + if (_consumerIsWaiting && !IsChunkBufferEmpty) { TraceHelpers.TraceInfo("AsynchronousChannel::EnqueueChunk - producer waking consumer"); Debug.Assert(_consumerEvent != null); - _consumerIsWaiting = 0; + _consumerIsWaiting = false; _consumerEvent.Set(_index); } } @@ -341,7 +341,7 @@ private void WaitUntilNonFull() // very quickly, suddenly seeing an empty queue. This would lead to deadlock // if we aren't careful. Therefore we check the empty/full state AGAIN after // setting our flag to see if a real wait is warranted. - Interlocked.Exchange(ref _producerIsWaiting, 1); + Interlocked.Exchange(ref _producerIsWaiting, true); // (We have to prevent the reads that go into determining whether the buffer // is full from moving before the write to the producer-wait flag. Hence the CAS.) @@ -359,7 +359,7 @@ private void WaitUntilNonFull() else { // Reset the flags, we don't actually have to wait after all. - _producerIsWaiting = 0; + _producerIsWaiting = false; } } while (IsFull); @@ -558,7 +558,7 @@ private bool TryDequeueChunk([NotNullWhen(true)] ref T[]? chunk, ref bool isDone // very quickly, suddenly seeing a full queue. This would lead to deadlock // if we aren't careful. Therefore we check the empty/full state AGAIN after // setting our flag to see if a real wait is warranted. - Interlocked.Exchange(ref _consumerIsWaiting, 1); + Interlocked.Exchange(ref _consumerIsWaiting, true); // (We have to prevent the reads that go into determining whether the buffer // is full from moving before the write to the producer-wait flag. Hence the CAS.) @@ -580,7 +580,7 @@ private bool TryDequeueChunk([NotNullWhen(true)] ref T[]? chunk, ref bool isDone { // Reset the wait flags, we don't need to wait after all. We loop back around // and recheck that the queue isn't empty, done, etc. - _consumerIsWaiting = 0; + _consumerIsWaiting = false; } } @@ -624,11 +624,11 @@ private T[] InternalDequeueChunk() // that the write to _consumerBufferIndex doesn't pass the read of the wait-flags; the CLR memory // model sadly permits this reordering. Hence the CAS above.) - if (_producerIsWaiting == 1 && !IsFull) + if (_producerIsWaiting && !IsFull) { TraceHelpers.TraceInfo("BoundedSingleLockFreeChannel::DequeueChunk - consumer waking producer"); Debug.Assert(_producerEvent != null); - _producerIsWaiting = 0; + _producerIsWaiting = false; _producerEvent.Set(); } @@ -643,7 +643,7 @@ private T[] InternalDequeueChunk() internal void DoneWithDequeueWait() { // On our way out, be sure to reset the flags. - _consumerIsWaiting = 0; + _consumerIsWaiting = false; } //----------------------------------------------------------------------------------- diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs index c00071ac3897cf..091c0e414d5b8f 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs @@ -38,10 +38,10 @@ internal sealed class Http3Connection : HttpConnectionBase // https://www.rfc-editor.org/rfc/rfc9114.html#section-7.2.4.1-2.2.1 private uint _maxHeaderListSize = uint.MaxValue; // Defaults to infinite - // Once the server's streams are received, these are set to 1. Further receipt of these streams results in a connection error. - private int _haveServerControlStream; - private int _haveServerQpackDecodeStream; - private int _haveServerQpackEncodeStream; + // Once the server's streams are received, these are set to true. Further receipt of these streams results in a connection error. + private bool _haveServerControlStream; + private bool _haveServerQpackDecodeStream; + private bool _haveServerQpackEncodeStream; // A connection-level error will abort any future operations. private Exception? _abortException; @@ -589,7 +589,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) switch (buffer.ActiveSpan[0]) { case (byte)Http3StreamType.Control: - if (Interlocked.Exchange(ref _haveServerControlStream, 1) != 0) + if (Interlocked.Exchange(ref _haveServerControlStream, true)) { // A second control stream has been received. throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.StreamCreationError); @@ -605,7 +605,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) await ProcessServerControlStreamAsync(stream, bufferCopy).ConfigureAwait(false); return; case (byte)Http3StreamType.QPackDecoder: - if (Interlocked.Exchange(ref _haveServerQpackDecodeStream, 1) != 0) + if (Interlocked.Exchange(ref _haveServerQpackDecodeStream, true)) { // A second QPack decode stream has been received. throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.StreamCreationError); @@ -616,7 +616,7 @@ private async Task ProcessServerStreamAsync(QuicStream stream) await stream.CopyToAsync(Stream.Null).ConfigureAwait(false); return; case (byte)Http3StreamType.QPackEncoder: - if (Interlocked.Exchange(ref _haveServerQpackEncodeStream, 1) != 0) + if (Interlocked.Exchange(ref _haveServerQpackEncodeStream, true)) { // A second QPack encode stream has been received. throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.StreamCreationError); diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs index c850389203d3d8..6262a6da33c238 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs @@ -67,8 +67,7 @@ internal sealed partial class HttpConnection : HttpConnectionBase private bool _canRetry; private bool _connectionClose; // Connection: close was seen on last response - private const int Status_Disposed = 1; - private int _disposed; + private volatile bool _disposed; public HttpConnection( HttpConnectionPool pool, @@ -97,7 +96,7 @@ private void Dispose(bool disposing) { // Ensure we're only disposed once. Dispose could be called concurrently, for example, // if the request and the response were running concurrently and both incurred an exception. - if (Interlocked.Exchange(ref _disposed, Status_Disposed) != Status_Disposed) + if (!Interlocked.Exchange(ref _disposed, true)) { if (NetEventSource.Log.IsEnabled()) Trace("Connection closing."); @@ -871,7 +870,7 @@ public async Task SendAsync(HttpRequestMessage request, boo // In case the connection is disposed, it's most probable that // expect100Continue timer expired and request content sending failed. // We're awaiting the task to propagate the exception in this case. - if (Volatile.Read(ref _disposed) == Status_Disposed) + if (_disposed) { try { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpContentReadStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpContentReadStream.cs index 02ba78ab99ca93..bfdbfb780fbbb2 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpContentReadStream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpContentReadStream.cs @@ -11,13 +11,13 @@ internal sealed partial class HttpConnection { internal abstract class HttpContentReadStream : HttpContentStream { - private int _disposed; // 0==no, 1==yes + private bool _disposed; public HttpContentReadStream(HttpConnection connection) : base(connection) { } - public sealed override bool CanRead => _disposed == 0; + public sealed override bool CanRead => !_disposed; public sealed override bool CanWrite => false; public sealed override void Write(ReadOnlySpan buffer) => throw new NotSupportedException(SR.net_http_content_readonly_stream); @@ -26,7 +26,7 @@ public HttpContentReadStream(HttpConnection connection) : base(connection) public virtual bool NeedsDrain => false; - protected bool IsDisposed => _disposed == 1; + protected bool IsDisposed => _disposed; protected bool CanReadFromConnection { @@ -35,7 +35,7 @@ protected bool CanReadFromConnection // _connection == null typically means that we have finished reading the response. // Cancellation may lead to a state where a disposed _connection is not null. HttpConnection? connection = _connection; - return connection != null && connection._disposed != Status_Disposed; + return connection != null && !connection._disposed; } } @@ -52,7 +52,7 @@ protected override void Dispose(bool disposing) // response stream and response content) will kick off multiple concurrent draining // operations. Also don't delegate to the base if Dispose has already been called, // as doing so will end up disposing of the connection before we're done draining. - if (Interlocked.Exchange(ref _disposed, 1) != 0) + if (Interlocked.Exchange(ref _disposed, true)) { return; } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs index 899fff5d788eb4..9315b0c511fb8e 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/HttpListener.Windows.cs @@ -1535,7 +1535,15 @@ private sealed class DisconnectAsyncResult : IAsyncResult private readonly ulong _connectionId; private readonly HttpListenerSession _listenerSession; private readonly NativeOverlapped* _nativeOverlapped; - private int _ownershipState; // 0 = normal, 1 = in HandleAuthentication(), 2 = disconnected, 3 = cleaned up + private OwnershipState _ownershipState; + + private enum OwnershipState + { + Normal, + InHandleAuthentication, + Disconnected, + CleanedUp + } internal NativeOverlapped* NativeOverlapped { @@ -1577,7 +1585,7 @@ public bool IsCompleted internal unsafe DisconnectAsyncResult(HttpListenerSession session, ulong connectionId) { if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(this, $"HttpListener: {session.Listener}, ConnectionId: {connectionId}"); - _ownershipState = 1; + _ownershipState = OwnershipState.InHandleAuthentication; _listenerSession = session; _connectionId = connectionId; @@ -1588,23 +1596,23 @@ internal unsafe DisconnectAsyncResult(HttpListenerSession session, ulong connect internal bool StartOwningDisconnectHandling() { - int oldValue; + OwnershipState oldValue; SpinWait spin = default; - while ((oldValue = Interlocked.CompareExchange(ref _ownershipState, 1, 0)) == 2) + while ((oldValue = Interlocked.CompareExchange(ref _ownershipState, OwnershipState.InHandleAuthentication, OwnershipState.Normal)) == OwnershipState.Disconnected) { // Must block until it equals 3 - we must be in the callback right now. spin.SpinOnce(); } - Debug.Assert(oldValue != 1, "StartOwningDisconnectHandling called twice."); - return oldValue < 2; + Debug.Assert(oldValue != OwnershipState.InHandleAuthentication, "StartOwningDisconnectHandling called twice."); + return oldValue < OwnershipState.Disconnected; } internal void FinishOwningDisconnectHandling() { // If it got disconnected, run the disconnect code. - if (Interlocked.CompareExchange(ref _ownershipState, 0, 1) == 2) + if (Interlocked.CompareExchange(ref _ownershipState, OwnershipState.Normal, OwnershipState.InHandleAuthentication) == OwnershipState.Disconnected) { HandleDisconnect(); } @@ -1620,7 +1628,7 @@ private static unsafe void IOCompleted(DisconnectAsyncResult asyncResult, Native if (NetEventSource.Log.IsEnabled()) NetEventSource.Info(null, "_connectionId:" + asyncResult._connectionId); asyncResult._listenerSession.RequestQueueBoundHandle.FreeNativeOverlapped(nativeOverlapped); - if (Interlocked.Exchange(ref asyncResult._ownershipState, 2) == 0) + if (Interlocked.Exchange(ref asyncResult._ownershipState, OwnershipState.Disconnected) == OwnershipState.Normal) { asyncResult.HandleDisconnect(); } @@ -1655,8 +1663,8 @@ private void HandleDisconnect() identity.Dispose(); } - int oldValue = Interlocked.Exchange(ref _ownershipState, 3); - Debug.Assert(oldValue == 2, $"Expected OwnershipState of 2, saw {oldValue}."); + OwnershipState oldValue = Interlocked.Exchange(ref _ownershipState, OwnershipState.CleanedUp); + Debug.Assert(oldValue == OwnershipState.Disconnected, $"Expected OwnershipState of Disconnected, saw {oldValue}."); } internal WindowsPrincipal? AuthenticatedConnection { get; set; } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBase.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBase.cs index c1d2e8f81324c5..982a4766538dfb 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBase.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBase.cs @@ -50,7 +50,7 @@ internal abstract class WebSocketBase : WebSocket, IDisposable private volatile WebSocketOperation.CloseOutputOperation? _closeOutputOperation; private Nullable _closeStatus; private string? _closeStatusDescription; - private int _receiveState; + private ReceiveState _receiveState; private Exception? _pendingException; protected WebSocketBase(Stream innerStream, @@ -1199,9 +1199,9 @@ private void ThrowIfDisposed() ObjectDisposedException.ThrowIf(_isDisposed, this); } - private void UpdateReceiveState(int newReceiveState, int expectedReceiveState) + private void UpdateReceiveState(ReceiveState newReceiveState, ReceiveState expectedReceiveState) { - int receiveState; + ReceiveState receiveState; if ((receiveState = Interlocked.Exchange(ref _receiveState, newReceiveState)) != expectedReceiveState) { Debug.Fail($"'_receiveState' had an invalid value '{receiveState}'. The expected value was '{expectedReceiveState}'."); @@ -1588,7 +1588,7 @@ protected virtual void ProcessAction_IndicateReceiveComplete( public sealed class ReceiveOperation : WebSocketOperation { - private int _receiveState; + private ReceiveState _receiveState; private bool _pongReceived; private bool _receiveCompleted; @@ -1614,8 +1614,7 @@ protected override void Initialize(Nullable> buffer, Cancella _receiveCompleted = false; _webSocket.ThrowIfDisposed(); - int originalReceiveState = Interlocked.CompareExchange(ref _webSocket._receiveState, - ReceiveState.Application, ReceiveState.Idle); + ReceiveState originalReceiveState = Interlocked.CompareExchange(ref _webSocket._receiveState, ReceiveState.Application, ReceiveState.Idle); switch (originalReceiveState) { @@ -1709,7 +1708,7 @@ protected override void ProcessAction_IndicateReceiveComplete( { ArraySegment payload; WebSocketMessageType messageType = GetMessageType(bufferType); - int newReceiveState = ReceiveState.Idle; + ReceiveState newReceiveState = ReceiveState.Idle; if (bufferType == WebSocketProtocolComponent.BufferType.Close) { @@ -2156,12 +2155,12 @@ internal interface IWebSocketStream Task CloseNetworkConnectionAsync(CancellationToken cancellationToken); } - private static class ReceiveState + private enum ReceiveState { - internal const int SendOperation = -1; - internal const int Idle = 0; - internal const int Application = 1; - internal const int PayloadAvailable = 2; + SendOperation = -1, + Idle = 0, + Application = 1, + PayloadAvailable = 2, } } } diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs index 245fb112d9314c..b1426b89dad4c8 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketBuffer.cs @@ -44,7 +44,7 @@ internal sealed class WebSocketBuffer : IDisposable private ArraySegment _pinnedSendBuffer; private GCHandle _pinnedSendBufferHandle; private int _stateWhenDisposing = int.MinValue; - private int _sendBufferState; + private SendBufferState _sendBufferState; private WebSocketBuffer(ArraySegment internalBuffer, int receiveBufferSize, int sendBufferSize) { @@ -170,7 +170,7 @@ internal void PinSendBuffer(ArraySegment payload, out bool bufferHasBeenPi { bufferHasBeenPinned = false; WebSocketValidate.ValidateBuffer(payload.Array!, payload.Offset, payload.Count); - int previousState = Interlocked.Exchange(ref _sendBufferState, SendBufferState.SendPayloadSpecified); + SendBufferState previousState = Interlocked.Exchange(ref _sendBufferState, SendBufferState.SendPayloadSpecified); if (previousState != SendBufferState.None) { @@ -274,7 +274,7 @@ internal bool IsPinnedSendPayloadBuffer(Interop.WebSocket.Buffer buffer, // This method is only thread safe for races between Abort and at most 1 uncompleted send operation internal void ReleasePinnedSendBuffer() { - int previousState = Interlocked.Exchange(ref _sendBufferState, SendBufferState.None); + SendBufferState previousState = Interlocked.Exchange(ref _sendBufferState, SendBufferState.None); if (previousState != SendBufferState.SendPayloadSpecified) { @@ -662,10 +662,10 @@ private static int GetInternalBufferSize(int receiveBufferSize, int sendBufferSi return 2 * receiveBufferSize + nativeSendBufferSize + NativeOverheadBufferSize + s_PropertyBufferSize; } - private static class SendBufferState + private enum SendBufferState { - public const int None = 0; - public const int SendPayloadSpecified = 1; + None = 0, + SendPayloadSpecified = 1, } private sealed class PayloadReceiveResult diff --git a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs index d694448435d1ad..229d4bcba99bd7 100644 --- a/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs +++ b/src/libraries/System.Net.HttpListener/src/System/Net/Windows/WebSockets/WebSocketHttpListenerDuplexStream.cs @@ -30,7 +30,7 @@ internal sealed class WebSocketHttpListenerDuplexStream : Stream, WebSocketBase. private HttpListenerAsyncEventArgs? _readEventArgs; private TaskCompletionSource? _writeTaskCompletionSource; private TaskCompletionSource? _readTaskCompletionSource; - private int _cleanedUp; + private bool _cleanedUp; #if DEBUG private sealed class OutstandingOperations @@ -595,7 +595,7 @@ public async Task CloseNetworkConnectionAsync(CancellationToken cancellationToke protected override void Dispose(bool disposing) { - if (disposing && Interlocked.Exchange(ref _cleanedUp, 1) == 0) + if (disposing && !Interlocked.Exchange(ref _cleanedUp, true)) { _readTaskCompletionSource?.TrySetCanceled(); @@ -716,10 +716,14 @@ private static void OnReadCompleted(object? sender, HttpListenerAsyncEventArgs e internal sealed class HttpListenerAsyncEventArgs : EventArgs, IDisposable { - private const int Free = 0; - private const int InProgress = 1; - private const int Disposed = 2; - private int _operating; + private OperatingState _operating; + + private enum OperatingState + { + Free = 0, + InProgress = 1, + Disposed = 2, + } private bool _disposeCalled; private unsafe NativeOverlapped* _ptrNativeOverlapped; @@ -783,7 +787,7 @@ public IList>? BufferList Debug.Assert(!_shouldCloseOutput, "'m_ShouldCloseOutput' MUST be 'false' at this point."); Debug.Assert(value == null || _buffer == null, "Either 'm_Buffer' or 'm_BufferList' MUST be NULL."); - Debug.Assert(_operating == Free, + Debug.Assert(_operating == OperatingState.Free, "This property can only be modified if no IO operation is outstanding."); Debug.Assert(value == null || value.Count == 2, "This list can only be 'NULL' or MUST have exactly '2' items."); @@ -883,7 +887,7 @@ public void Dispose() _disposeCalled = true; // Check if this object is in-use for an async socket operation. - if (Interlocked.CompareExchange(ref _operating, Disposed, Free) != Free) + if (Interlocked.CompareExchange(ref _operating, OperatingState.Disposed, OperatingState.Free) != OperatingState.Free) { // Either already disposed or will be disposed when current operation completes. return; @@ -930,7 +934,7 @@ private unsafe void FreeOverlapped(bool checkForShutdown) internal void StartOperationCommon(ThreadPoolBoundHandle boundHandle) { // Change status to "in-use". - if (Interlocked.CompareExchange(ref _operating, InProgress, Free) != Free) + if (Interlocked.CompareExchange(ref _operating, OperatingState.InProgress, OperatingState.Free) != OperatingState.Free) { // If it was already "in-use" check if Dispose was called. ObjectDisposedException.ThrowIf(_disposeCalled, this); @@ -1039,7 +1043,7 @@ internal void Complete() { FreeOverlapped(false); // Mark as not in-use - Interlocked.Exchange(ref _operating, Free); + Interlocked.Exchange(ref _operating, OperatingState.Free); // Check for deferred Dispose(). // The deferred Dispose is not guaranteed if Dispose is called while an operation is in progress. diff --git a/src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpClient.cs b/src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpClient.cs index 26a496f87a635e..884d3b18a38669 100644 --- a/src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpClient.cs +++ b/src/libraries/System.Net.Mail/src/System/Net/Mail/SmtpClient.cs @@ -739,7 +739,7 @@ public Task SendMailAsync(MailMessage message, CancellationToken cancellationTok CancellationTokenRegistration ctr = default; // Indicates whether the CTR has been set - captured in handler - int state = 0; + bool ctrSet = false; // Register a handler that will transfer completion results to the TCS Task SendCompletedEventHandler? handler = null; @@ -750,7 +750,7 @@ public Task SendMailAsync(MailMessage message, CancellationToken cancellationTok try { ((SmtpClient)sender).SendCompleted -= handler; - if (Interlocked.Exchange(ref state, 1) != 0) + if (Interlocked.Exchange(ref ctrSet, true)) { // A CTR has been set, we have to wait until it completes before completing the task ctr.Dispose(); @@ -783,7 +783,7 @@ public Task SendMailAsync(MailMessage message, CancellationToken cancellationTok ((SmtpClient)s!).SendAsyncCancel(); }, this); - if (Interlocked.Exchange(ref state, 1) != 0) + if (Interlocked.Exchange(ref ctrSet, true)) { // SendCompleted was already invoked, ensure the CTR completes before returning the task ctr.Dispose(); diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs index 46325cf2e14867..e6914f2a19cf81 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicConnection.cs @@ -109,9 +109,9 @@ static async ValueTask StartConnectAsync(QuicClientConnectionOpt private readonly MsQuicContextSafeHandle _handle; /// - /// Set to non-zero once disposed. Prevents double and/or concurrent disposal. + /// Set to true once disposed. Prevents double and/or concurrent disposal. /// - private int _disposed; + private bool _disposed; /// /// Completed when connection shutdown is initiated. @@ -502,7 +502,7 @@ private void DecrementStreamCapacity(QuicStreamType streamType) /// An asynchronous task that completes with the opened . public async ValueTask OpenOutboundStreamAsync(QuicStreamType type, CancellationToken cancellationToken = default) { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); QuicStream? stream = null; try @@ -524,7 +524,7 @@ public async ValueTask OpenOutboundStreamAsync(QuicStreamType type, } // Propagate ODE if disposed in the meantime. - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); // In case of an incoming race when the connection is closed by the peer just before we open the stream, // we receive QUIC_STATUS_ABORTED from MsQuic, but we don't know how the connection was closed. We throw @@ -548,7 +548,7 @@ public async ValueTask OpenOutboundStreamAsync(QuicStreamType type, /// An asynchronous task that completes with the accepted . public async ValueTask AcceptInboundStreamAsync(CancellationToken cancellationToken = default) { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); if (!_canAccept) { @@ -587,7 +587,7 @@ public async ValueTask AcceptInboundStreamAsync(CancellationToken ca /// An asynchronous task that completes when the connection is closed. public ValueTask CloseAsync(long errorCode, CancellationToken cancellationToken = default) { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); ThrowHelper.ValidateErrorCode(nameof(errorCode), errorCode, $"{nameof(CloseAsync)}.{nameof(errorCode)}"); if (_shutdownTcs.TryGetValueTask(out ValueTask valueTask, this, cancellationToken)) @@ -649,7 +649,7 @@ private unsafe int HandleEventShutdownComplete() // make sure we log at least some secrets in case of shutdown before handshake completes. _tlsSecret?.WriteSecret(); - Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(_disposed == 1 ? new ObjectDisposedException(GetType().FullName) : ThrowHelper.GetOperationAbortedException()); + Exception exception = ExceptionDispatchInfo.SetCurrentStackTrace(_disposed ? new ObjectDisposedException(GetType().FullName) : ThrowHelper.GetOperationAbortedException()); _connectionCloseTcs.TrySetException(exception); _acceptQueue.Writer.TryComplete(exception); _connectedTcs.TrySetException(exception); @@ -787,7 +787,7 @@ private static unsafe int NativeCallback(QUIC_HANDLE* connection, void* context, /// A task that represents the asynchronous dispose operation. public async ValueTask DisposeAsync() { - if (Interlocked.Exchange(ref _disposed, 1) != 0) + if (Interlocked.Exchange(ref _disposed, true)) { return; } diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs index 7e8acfda32ccb1..92733f4bb39638 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicListener.cs @@ -74,9 +74,9 @@ public static ValueTask ListenAsync(QuicListenerOptions options, C private readonly MsQuicContextSafeHandle _handle; /// - /// Set to non-zero once disposed. Prevents double and/or concurrent disposal. + /// Set to true once disposed. Prevents double and/or concurrent disposal. /// - private int _disposed; + private bool _disposed; /// /// Completed when SHUTDOWN_COMPLETE arrives. @@ -175,7 +175,7 @@ private unsafe QuicListener(QuicListenerOptions options) /// A task that will contain a fully connected which successfully finished the handshake and is ready to be used. public async ValueTask AcceptConnectionAsync(CancellationToken cancellationToken = default) { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); GCHandle keepObject = GCHandle.Alloc(this); try @@ -405,7 +405,7 @@ private static unsafe int NativeCallback(QUIC_HANDLE* listener, void* context, Q /// A task that represents the asynchronous dispose operation. public async ValueTask DisposeAsync() { - if (Interlocked.Exchange(ref _disposed, 1) != 0) + if (Interlocked.Exchange(ref _disposed, true)) { return; } diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.Stream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.Stream.cs index 8196c59a1c1bda..42824723a950ce 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.Stream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.Stream.cs @@ -50,12 +50,12 @@ public override int ReadTimeout { get { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); return (int)_readTimeout.TotalMilliseconds; } set { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); if (value <= 0 && value != Timeout.Infinite) { throw new ArgumentOutOfRangeException(nameof(value), SR.net_quic_timeout_use_gt_zero); @@ -69,12 +69,12 @@ public override int WriteTimeout { get { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); return (int)_writeTimeout.TotalMilliseconds; } set { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); if (value <= 0 && value != Timeout.Infinite) { throw new ArgumentOutOfRangeException(nameof(value), SR.net_quic_timeout_use_gt_zero); @@ -86,7 +86,7 @@ public override int WriteTimeout // Read boilerplate. /// /// Gets a value indicating whether the supports reading. - public override bool CanRead => Volatile.Read(ref _disposed) == 0 && _canRead; + public override bool CanRead => !Volatile.Read(ref _disposed) && _canRead; /// public override IAsyncResult BeginRead(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) @@ -113,7 +113,7 @@ public override int ReadByte() /// public override int Read(Span buffer) { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); byte[] rentedBuffer = ArrayPool.Shared.Rent(buffer.Length); CancellationTokenSource? cts = null; @@ -149,7 +149,7 @@ public override Task ReadAsync(byte[] buffer, int offset, int count, Cancel // Write boilerplate. /// /// Gets a value indicating whether the supports writing. - public override bool CanWrite => Volatile.Read(ref _disposed) == 0 && _canWrite; + public override bool CanWrite => !Volatile.Read(ref _disposed) && _canWrite; /// public override IAsyncResult BeginWrite(byte[] buffer, int offset, int count, AsyncCallback? callback, object? state) @@ -175,7 +175,7 @@ public override void WriteByte(byte value) /// public override void Write(ReadOnlySpan buffer) { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); CancellationTokenSource? cts = null; if (_writeTimeout > TimeSpan.Zero) diff --git a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs index a0713d3b8f9bb8..adbe79e9c40d85 100644 --- a/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs +++ b/src/libraries/System.Net.Quic/src/System/Net/Quic/QuicStream.cs @@ -60,9 +60,9 @@ public sealed partial class QuicStream private readonly MsQuicContextSafeHandle _handle; /// - /// Set to non-zero once disposed. Prevents double and/or concurrent disposal. + /// Set to true once disposed. Prevents double and/or concurrent disposal. /// - private int _disposed; + private bool _disposed; private readonly ValueTaskSource _startedTcs = new ValueTaskSource(); private readonly ValueTaskSource _shutdownTcs = new ValueTaskSource(); @@ -275,7 +275,7 @@ internal ValueTask StartAsync(Action decrementStreamCapacity, Ca /// public override async ValueTask ReadAsync(Memory buffer, CancellationToken cancellationToken = default) { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); if (!_canRead) { @@ -362,7 +362,7 @@ public override ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationTo /// Notifies the peer about gracefully closing the write side, i.e.: sends FIN flag with the data. public ValueTask WriteAsync(ReadOnlyMemory buffer, bool completeWrites, CancellationToken cancellationToken = default) { - if (_disposed == 1) + if (_disposed) { return ValueTask.FromException(ExceptionDispatchInfo.SetCurrentStackTrace(new ObjectDisposedException(nameof(QuicStream)))); } @@ -454,7 +454,7 @@ public ValueTask WriteAsync(ReadOnlyMemory buffer, bool completeWrites, Ca /// The error code with which to abort the stream, this value is application protocol (layer above QUIC) dependent. public void Abort(QuicAbortDirection abortDirection, long errorCode) { - if (_disposed == 1) + if (_disposed) { return; } @@ -513,7 +513,7 @@ public void Abort(QuicAbortDirection abortDirection, long errorCode) /// public void CompleteWrites() { - ObjectDisposedException.ThrowIf(_disposed == 1, this); + ObjectDisposedException.ThrowIf(_disposed, this); // Nothing to complete, the writing side is already closed. if (_sendTcs.IsCompleted) @@ -715,7 +715,7 @@ private static unsafe int NativeCallback(QUIC_HANDLE* stream, void* context, QUI /// A task that represents the asynchronous dispose operation. public override async ValueTask DisposeAsync() { - if (Interlocked.Exchange(ref _disposed, 1) != 0) + if (Interlocked.Exchange(ref _disposed, true)) { return; } diff --git a/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs b/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs index fbc32738ef21d1..ee2466d285a142 100644 --- a/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs +++ b/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs @@ -51,10 +51,10 @@ public class HttpWebRequest : WebRequest, ISerializable private static int _defaultMaxResponseHeadersLength = HttpHandlerDefaults.DefaultMaxResponseHeadersLength; private static int _defaultMaximumErrorResponseLength = -1; - private int _beginGetRequestStreamCalled; - private int _beginGetResponseCalled; - private int _endGetRequestStreamCalled; - private int _endGetResponseCalled; + private bool _beginGetRequestStreamCalled; + private bool _beginGetResponseCalled; + private bool _endGetRequestStreamCalled; + private bool _endGetResponseCalled; private int _maximumAllowedRedirections = HttpHandlerDefaults.DefaultMaxAutomaticRedirections; private int _maximumResponseHeadersLen = _defaultMaxResponseHeadersLength; @@ -74,7 +74,7 @@ public class HttpWebRequest : WebRequest, ISerializable private TaskCompletionSource? _responseOperation; private AsyncCallback? _requestStreamCallback; private AsyncCallback? _responseCallback; - private int _abortCalled; + private volatile bool _abortCalled; private CancellationTokenSource? _sendRequestCts; private X509CertificateCollection? _clientCertificates; private Booleans _booleans = Booleans.Default; @@ -995,7 +995,7 @@ public override IWebProxy? Proxy public override void Abort() { - if (Interlocked.Exchange(ref _abortCalled, 1) != 0) + if (Interlocked.Exchange(ref _abortCalled, true)) { return; } @@ -1125,7 +1125,7 @@ public override IAsyncResult BeginGetRequestStream(AsyncCallback? callback, obje { CheckAbort(); - if (Interlocked.Exchange(ref _beginGetRequestStreamCalled, 1) != 0) + if (Interlocked.Exchange(ref _beginGetRequestStreamCalled, true)) { throw new InvalidOperationException(SR.net_repcall); } @@ -1147,7 +1147,7 @@ public override Stream EndGetRequestStream(IAsyncResult asyncResult) throw new ArgumentException(SR.net_io_invalidasyncresult, nameof(asyncResult)); } - if (Interlocked.Exchange(ref _endGetRequestStreamCalled, 1) != 0) + if (Interlocked.Exchange(ref _endGetRequestStreamCalled, true)) { throw new InvalidOperationException(SR.Format(SR.net_io_invalidendcall, "EndGetRequestStream")); } @@ -1398,7 +1398,7 @@ public override IAsyncResult BeginGetResponse(AsyncCallback? callback, object? s { CheckAbort(); - if (Interlocked.Exchange(ref _beginGetResponseCalled, 1) != 0) + if (Interlocked.Exchange(ref _beginGetResponseCalled, true)) { throw new InvalidOperationException(SR.net_repcall); } @@ -1418,7 +1418,7 @@ public override WebResponse EndGetResponse(IAsyncResult asyncResult) throw new ArgumentException(SR.net_io_invalidasyncresult, nameof(asyncResult)); } - if (Interlocked.Exchange(ref _endGetResponseCalled, 1) != 0) + if (Interlocked.Exchange(ref _endGetResponseCalled, true)) { throw new InvalidOperationException(SR.Format(SR.net_io_invalidendcall, "EndGetResponse")); } @@ -1565,7 +1565,7 @@ private bool RequestSubmitted private void CheckAbort() { - if (Volatile.Read(ref _abortCalled) == 1) + if (_abortCalled) { throw new WebException(SR.net_reqaborted, WebExceptionStatus.RequestCanceled); } diff --git a/src/libraries/System.Net.Requests/src/System/Net/TimerThread.cs b/src/libraries/System.Net.Requests/src/System/Net/TimerThread.cs index 3aef8b469fded8..4bbfd0f4bb8a54 100644 --- a/src/libraries/System.Net.Requests/src/System/Net/TimerThread.cs +++ b/src/libraries/System.Net.Requests/src/System/Net/TimerThread.cs @@ -445,14 +445,14 @@ private sealed class InfiniteTimer : Timer { internal InfiniteTimer() : base(Timeout.Infinite) { } - private int _cancelled; + private bool _canceled; internal override bool HasExpired => false; /// /// Cancels the timer. Returns true the first time, false after that. /// - internal override bool Cancel() => Interlocked.Exchange(ref _cancelled, 1) == 0; + internal override bool Cancel() => !Interlocked.Exchange(ref _canceled, true); } /// diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs index 48e3923037cc48..b0f25d431b2b90 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/NegotiateStream.cs @@ -42,9 +42,9 @@ public partial class NegotiateStream : AuthenticatedStream private int _readBufferCount; private ArrayBufferWriter? _writeBuffer; - private volatile int _writeInProgress; - private volatile int _readInProgress; - private volatile int _authInProgress; + private volatile bool _writeInProgress; + private volatile bool _readInProgress; + private volatile bool _authInProgress; private ExceptionDispatchInfo? _exception; private StreamFramer? _framer; @@ -340,7 +340,7 @@ private async ValueTask ReadAsync(Memory buffer, Cancella { Debug.Assert(_context is not null); - if (Interlocked.Exchange(ref _readInProgress, 1) == 1) + if (Interlocked.Exchange(ref _readInProgress, true)) { throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, "read")); } @@ -441,7 +441,7 @@ private async ValueTask ReadAsync(Memory buffer, Cancella } finally { - _readInProgress = 0; + _readInProgress = false; } static async ValueTask ReadAllAsync(Stream stream, Memory buffer, bool allowZeroRead, CancellationToken cancellationToken) @@ -506,7 +506,7 @@ private async Task WriteAsync(ReadOnlyMemory buffer, Cancellat Debug.Assert(_context is not null); Debug.Assert(_writeBuffer is not null); - if (Interlocked.Exchange(ref _writeInProgress, 1) == 1) + if (Interlocked.Exchange(ref _writeInProgress, true)) { throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, "write")); } @@ -556,7 +556,7 @@ private async Task WriteAsync(ReadOnlyMemory buffer, Cancellat finally { _writeBuffer.Clear(); - _writeInProgress = 0; + _writeInProgress = false; } } @@ -724,7 +724,7 @@ private async Task AuthenticateAsync(CancellationToken cancellationT Debug.Assert(_context != null); ThrowIfFailed(authSuccessCheck: false); - if (Interlocked.Exchange(ref _authInProgress, 1) == 1) + if (Interlocked.Exchange(ref _authInProgress, true)) { throw new InvalidOperationException(SR.Format(SR.net_io_invalidnestedcall, "authenticate")); } @@ -742,7 +742,7 @@ private async Task AuthenticateAsync(CancellationToken cancellationT } finally { - _authInProgress = 0; + _authInProgress = false; } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs index 5a01ee065c2b54..2e8262b5a7ff28 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.IO.cs @@ -16,7 +16,7 @@ namespace System.Net.Security public partial class SslStream { private readonly SslAuthenticationOptions _sslAuthenticationOptions = new SslAuthenticationOptions(); - private int _nestedAuth; + private NestedState _nestedAuth; private bool _isRenego; private TlsFrameHelper.TlsFrameInfo _lastFrame; @@ -32,10 +32,14 @@ public partial class SslStream private bool _receivedEOF; // Used by Telemetry to ensure we log connection close exactly once - // 0 = no handshake - // 1 = handshake completed, connection opened - // 2 = SslStream disposed, connection closed - private int _connectionOpenedStatus; + private enum ConnectionStatus + { + NoHandshake = 0, + HandshakeCompleted = 1, // connection opened + Disposed = 2, // connection closed + } + + private ConnectionStatus _connectionOpenedStatus; private void SetException(Exception e) { @@ -56,10 +60,10 @@ private void CloseInternal() // Ensure a Read or Auth operation is not in progress, // block potential future read and auth operations since SslStream is disposing. - // This leaves the _nestedRead = 2 and _nestedAuth = 2, but that's ok, since + // This leaves the _nestedRead = StreamDisposed and _nestedAuth = StreamDisposed, but that's ok, since // subsequent operations check the _exception sentinel first - if (Interlocked.Exchange(ref _nestedRead, StreamDisposed) == StreamNotInUse && - Interlocked.Exchange(ref _nestedAuth, StreamDisposed) == StreamNotInUse) + if (Interlocked.Exchange(ref _nestedRead, NestedState.StreamDisposed) == NestedState.StreamNotInUse && + Interlocked.Exchange(ref _nestedAuth, NestedState.StreamDisposed) == NestedState.StreamNotInUse) { _buffer.ReturnBuffer(); } @@ -73,7 +77,7 @@ private void CloseInternal() if (NetSecurityTelemetry.Log.IsEnabled()) { // Set the status to disposed. If it was opened before, log ConnectionClosed - if (Interlocked.Exchange(ref _connectionOpenedStatus, 2) == 1) + if (Interlocked.Exchange(ref _connectionOpenedStatus, ConnectionStatus.Disposed) == ConnectionStatus.HandshakeCompleted) { NetSecurityTelemetry.Log.ConnectionClosed(GetSslProtocolInternal()); } @@ -133,7 +137,7 @@ private async Task ProcessAuthenticationWithTelemetryAsync(bool isAsync, Cancell // SslStream could already have been disposed at this point, in which case _connectionOpenedStatus == 2 // Make sure that we increment the open connection counter only if it is guaranteed to be decremented in dispose/finalize - bool connectionOpen = Interlocked.CompareExchange(ref _connectionOpenedStatus, 1, 0) == 0; + bool connectionOpen = Interlocked.CompareExchange(ref _connectionOpenedStatus, ConnectionStatus.HandshakeCompleted, ConnectionStatus.NoHandshake) == ConnectionStatus.NoHandshake; NetSecurityTelemetry.Log.HandshakeCompleted(GetSslProtocolInternal(), startingTimestamp, connectionOpen); } @@ -165,22 +169,22 @@ private async Task ReplyOnReAuthenticationAsync(byte[]? buffer, Canc private async Task RenegotiateAsync(CancellationToken cancellationToken) where TIOAdapter : IReadWriteAdapter { - if (Interlocked.CompareExchange(ref _nestedAuth, StreamInUse, StreamNotInUse) != StreamNotInUse) + if (Interlocked.CompareExchange(ref _nestedAuth, NestedState.StreamInUse, NestedState.StreamNotInUse) != NestedState.StreamNotInUse) { - ObjectDisposedException.ThrowIf(_nestedAuth == StreamDisposed, this); + ObjectDisposedException.ThrowIf(_nestedAuth == NestedState.StreamDisposed, this); throw new InvalidOperationException(SR.Format(SR.net_io_invalidnestedcall, "authenticate")); } - if (Interlocked.CompareExchange(ref _nestedRead, StreamInUse, StreamNotInUse) != StreamNotInUse) + if (Interlocked.CompareExchange(ref _nestedRead, NestedState.StreamInUse, NestedState.StreamNotInUse) != NestedState.StreamNotInUse) { - ObjectDisposedException.ThrowIf(_nestedRead == StreamDisposed, this); + ObjectDisposedException.ThrowIf(_nestedRead == NestedState.StreamDisposed, this); throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, "read")); } // Write is different since we do not do anything special in Dispose - if (Interlocked.Exchange(ref _nestedWrite, StreamInUse) != StreamNotInUse) + if (Interlocked.Exchange(ref _nestedWrite, NestedState.StreamInUse) != NestedState.StreamNotInUse) { - _nestedRead = StreamNotInUse; + _nestedRead = NestedState.StreamNotInUse; throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, "write")); } @@ -243,8 +247,8 @@ private async Task RenegotiateAsync(CancellationToken cancellationTo token.ReleasePayload(); - _nestedRead = StreamNotInUse; - _nestedWrite = StreamNotInUse; + _nestedRead = NestedState.StreamNotInUse; + _nestedWrite = NestedState.StreamNotInUse; _isRenego = false; // We will not release _nestedAuth at this point to prevent another renegotiation attempt. } @@ -262,7 +266,7 @@ private async Task ForceAuthenticationAsync(bool receiveFirst, byte[ if (reAuthenticationData == null) { // prevent nesting only when authentication functions are called explicitly. e.g. handle renegotiation transparently. - if (Interlocked.Exchange(ref _nestedAuth, StreamInUse) == StreamInUse) + if (Interlocked.Exchange(ref _nestedAuth, NestedState.StreamInUse) == NestedState.StreamInUse) { throw new InvalidOperationException(SR.Format(SR.net_io_invalidnestedcall, "authenticate")); } @@ -355,7 +359,7 @@ private async Task ForceAuthenticationAsync(bool receiveFirst, byte[ { if (reAuthenticationData == null) { - _nestedAuth = StreamNotInUse; + _nestedAuth = NestedState.StreamNotInUse; _isRenego = false; } @@ -526,7 +530,7 @@ private bool CompleteHandshake(ref ProtocolToken alertToken, out SslPolicyErrors { ProcessHandshakeSuccess(); - if (_nestedAuth != StreamInUse) + if (_nestedAuth != NestedState.StreamInUse) { if (NetEventSource.Log.IsEnabled()) NetEventSource.Error(this, $"Ignoring unsolicited renegotiated certificate."); // ignore certificates received outside of handshake or requested renegotiation. @@ -776,7 +780,7 @@ private SecurityStatusPal DecryptData(int frameSize) // If that happen before EncryptData() runs, _handshakeWaiter will be set to null // and EncryptData() will work normally e.g. no waiting, just exclusion with DecryptData() - if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != 0) + if (_sslAuthenticationOptions.AllowRenegotiation || SslProtocol == SslProtocols.Tls13 || _nestedAuth != NestedState.StreamNotInUse) { // create TCS only if we plan to proceed. If not, we will throw later outside of the lock. // Tls1.3 does not have renegotiation. However on Windows this error code is used @@ -799,9 +803,9 @@ private async ValueTask ReadAsyncInternal(Memory buffer, // Check for disposal is not atomic so we will check again below. ThrowIfExceptionalOrNotAuthenticated(); - if (Interlocked.CompareExchange(ref _nestedRead, StreamInUse, StreamNotInUse) != StreamNotInUse) + if (Interlocked.CompareExchange(ref _nestedRead, NestedState.StreamInUse, NestedState.StreamNotInUse) != NestedState.StreamNotInUse) { - ObjectDisposedException.ThrowIf(_nestedRead == StreamDisposed, this); + ObjectDisposedException.ThrowIf(_nestedRead == NestedState.StreamDisposed, this); throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, "read")); } @@ -926,7 +930,7 @@ private async ValueTask ReadAsyncInternal(Memory buffer, finally { ReturnReadBufferIfEmpty(); - _nestedRead = StreamNotInUse; + _nestedRead = NestedState.StreamNotInUse; } } @@ -941,7 +945,7 @@ private async ValueTask WriteAsyncInternal(ReadOnlyMemory buff return; } - if (Interlocked.Exchange(ref _nestedWrite, StreamInUse) == StreamInUse) + if (Interlocked.Exchange(ref _nestedWrite, NestedState.StreamInUse) == NestedState.StreamInUse) { throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, "write")); } @@ -964,7 +968,7 @@ private async ValueTask WriteAsyncInternal(ReadOnlyMemory buff } finally { - _nestedWrite = StreamNotInUse; + _nestedWrite = NestedState.StreamNotInUse; } } diff --git a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs index 82ca35d1b0d5ff..680f1649532725 100644 --- a/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs +++ b/src/libraries/System.Net.Security/src/System/Net/Security/SslStream.cs @@ -167,13 +167,15 @@ public void ReturnBuffer() } } - // used to track ussage in _nested* variables bellow - private const int StreamNotInUse = 0; - private const int StreamInUse = 1; - private const int StreamDisposed = 2; + private enum NestedState + { + StreamNotInUse = 0, + StreamInUse = 1, + StreamDisposed = 2, + } - private int _nestedWrite; - private int _nestedRead; + private NestedState _nestedWrite; + private NestedState _nestedRead; private PoolingPointerMemoryManager? _readPointerMemoryManager; private PoolingPointerMemoryManager? _writePointerMemoryManager; @@ -735,7 +737,7 @@ private static unsafe void ReturnPointerMemoryManager(ref PoolingPointerMemoryMa public override int ReadByte() { ThrowIfExceptionalOrNotAuthenticated(); - if (Interlocked.Exchange(ref _nestedRead, StreamInUse) == StreamInUse) + if (Interlocked.Exchange(ref _nestedRead, NestedState.StreamInUse) == NestedState.StreamInUse) { throw new NotSupportedException(SR.Format(SR.net_io_invalidnestedcall, "read")); } @@ -756,7 +758,7 @@ public override int ReadByte() // Regardless of whether we were able to read a byte from the buffer, // reset the read tracking. If we weren't able to read a byte, the // subsequent call to Read will set the flag again. - _nestedRead = StreamNotInUse; + _nestedRead = NestedState.StreamNotInUse; } // Otherwise, fall back to reading a byte via Read, the same way Stream.ReadByte does. diff --git a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs index 3ad6be920392ac..31346cc9715e52 100644 --- a/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs +++ b/src/libraries/System.Net.Security/tests/UnitTests/Fakes/FakeSslStream.Implementation.cs @@ -54,12 +54,12 @@ private void ValidateCreateContext(SslClientAuthenticationOptions sslClientAuthe // Without setting (or using) these members you will get a build exception in the unit test project. // The code that normally uses these in the main solution is in the implementation of SslStream. - if (_nestedWrite == 0) + if (_nestedWrite == NestedState.StreamNotInUse) { } _exception = null; - _nestedWrite = 0; + _nestedWrite = NestedState.StreamNotInUse; _handshakeCompleted = false; } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs index 9cb523300e849a..4b94be9b1df571 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/NetworkStream.cs @@ -23,8 +23,8 @@ public class NetworkStream : Stream // Used by the class to indicate that the stream is writable. private bool _writeable; - // Whether Dispose has been called. 0 == false, 1 == true - private int _disposed; + // Whether Dispose has been called. + private bool _disposed; // Creates a new instance of the System.Net.Sockets.NetworkStream class for the specified System.Net.Sockets.Socket. public NetworkStream(Socket socket) @@ -369,7 +369,7 @@ private static int ToTimeoutMilliseconds(TimeSpan timeout) protected override void Dispose(bool disposing) { - if (Interlocked.Exchange(ref _disposed, 1) != 0) + if (Interlocked.Exchange(ref _disposed, true)) { return; } @@ -685,7 +685,7 @@ internal void SetSocketTimeoutOption(SocketShutdown mode, int timeout, bool sile private void ThrowIfDisposed() { - ObjectDisposedException.ThrowIf(_disposed != 0, this); + ObjectDisposedException.ThrowIf(_disposed, this); } private static IOException WrapException(string resourceFormatString, Exception innerException) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs index f427aa88cc1f4b..7f653dab759b80 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SafeSocketHandle.cs @@ -28,7 +28,7 @@ public sealed partial class SafeSocketHandle : SafeHandleMinusOneIsInvalid private int _closeSocketThread; private int _closeSocketTick; #endif - private int _ownClose; + private bool _ownClose; /// /// Creates a . @@ -54,8 +54,7 @@ public SafeSocketHandle(IntPtr preexistingHandle, bool ownsHandle) internal bool HasShutdownSend => _hasShutdownSend; - private bool TryOwnClose() - => Interlocked.CompareExchange(ref _ownClose, 1, 0) == 0; + private bool TryOwnClose() => !Interlocked.Exchange(ref _ownClose, true); private volatile bool _released; private bool _hasShutdownSend; diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs index 73ff6521aa31fa..3e4650f547c483 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.Unix.cs @@ -148,7 +148,7 @@ internal SocketError ReplaceHandle() return errorCode; } - if (Volatile.Read(ref _disposed) != 0) + if (Volatile.Read(ref _disposed)) { _handle.Dispose(); throw new ObjectDisposedException(GetType().FullName); diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs index c8df9f89c4a033..5bfd297249adba 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/Socket.cs @@ -67,7 +67,7 @@ public partial class Socket : IDisposable private bool _receivingPacketInformation; private int _closeTimeout = Socket.DefaultCloseTimeout; - private int _disposed; // 0 == false, anything else == true + private bool _disposed; public Socket(SocketType socketType, ProtocolType protocolType) : this(OSSupportsIPv6 ? AddressFamily.InterNetworkV6 : AddressFamily.InterNetwork, socketType, protocolType) @@ -3146,7 +3146,7 @@ private bool SendToAsync(SocketAsyncEventArgs e, CancellationToken cancellationT // Internal and private properties // - internal bool Disposed => _disposed != 0; + internal bool Disposed => _disposed; // // Internal and private methods @@ -3238,7 +3238,7 @@ protected virtual void Dispose(bool disposing) } // Make sure we're the first call to Dispose - if (Interlocked.CompareExchange(ref _disposed, 1, 0) == 1) + if (Interlocked.Exchange(ref _disposed, true)) { return; } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs index dd50676ac55c46..d5613dc91f481a 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncContext.Unix.cs @@ -119,10 +119,10 @@ private enum State Canceled } - private int _state; // Actually AsyncOperation.State. + private volatile AsyncOperation.State _state; #if DEBUG - private int _callbackQueued; // When non-zero, the callback has been queued. + private bool _callbackQueued; // When true, the callback has been queued. #endif public readonly SocketAsyncContext AssociatedContext; @@ -141,11 +141,11 @@ public AsyncOperation(SocketAsyncContext context) public void Reset() { - _state = (int)State.Waiting; + _state = State.Waiting; Event = null; Next = this; #if DEBUG - _callbackQueued = 0; + _callbackQueued = false; #endif } @@ -154,34 +154,34 @@ public OperationResult TryComplete(SocketAsyncContext context) TraceWithContext(context, "Enter"); // Set state to Running, unless we've been canceled - int oldState = Interlocked.CompareExchange(ref _state, (int)State.Running, (int)State.Waiting); - if (oldState == (int)State.Canceled) + State oldState = Interlocked.CompareExchange(ref _state, State.Running, State.Waiting); + if (oldState == State.Canceled) { TraceWithContext(context, "Exit, Previously canceled"); return OperationResult.Cancelled; } - Debug.Assert(oldState == (int)State.Waiting, $"Unexpected operation state: {(State)oldState}"); + Debug.Assert(oldState == State.Waiting, $"Unexpected operation state: {(State)oldState}"); // Try to perform the IO if (DoTryComplete(context)) { - Debug.Assert((State)Volatile.Read(ref _state) is State.Running or State.RunningWithPendingCancellation, "Unexpected operation state"); + Debug.Assert(_state is State.Running or State.RunningWithPendingCancellation, "Unexpected operation state"); - Volatile.Write(ref _state, (int)State.Complete); + _state = State.Complete; TraceWithContext(context, "Exit, Completed"); return OperationResult.Completed; } // Set state back to Waiting, unless we were canceled, in which case we have to process cancellation now - int newState; + State newState; while (true) { - int state = Volatile.Read(ref _state); - Debug.Assert(state is (int)State.Running or (int)State.RunningWithPendingCancellation, $"Unexpected operation state: {(State)state}"); + State state = _state; + Debug.Assert(state is State.Running or State.RunningWithPendingCancellation, $"Unexpected operation state: {(State)state}"); - newState = (state == (int)State.Running ? (int)State.Waiting : (int)State.Canceled); + newState = (state == State.Running ? State.Waiting : State.Canceled); if (state == Interlocked.CompareExchange(ref _state, newState, state)) { break; @@ -190,7 +190,7 @@ public OperationResult TryComplete(SocketAsyncContext context) // Race to update the state. Loop and try again. } - if (newState == (int)State.Canceled) + if (newState == State.Canceled) { ProcessCancellation(); TraceWithContext(context, "Exit, Newly cancelled"); @@ -208,16 +208,16 @@ public bool TryCancel() // Note we could be cancelling because of socket close. Regardless, we don't need the registration anymore. CancellationRegistration.Dispose(); - int newState; + State newState; while (true) { - int state = Volatile.Read(ref _state); - if (state is (int)State.Complete or (int)State.Canceled or (int)State.RunningWithPendingCancellation) + State state = _state; + if (state is State.Complete or State.Canceled or State.RunningWithPendingCancellation) { return false; } - newState = (state == (int)State.Waiting ? (int)State.Canceled : (int)State.RunningWithPendingCancellation); + newState = (state == State.Waiting ? State.Canceled : State.RunningWithPendingCancellation); if (state == Interlocked.CompareExchange(ref _state, newState, state)) { break; @@ -226,7 +226,7 @@ public bool TryCancel() // Race to update the state. Loop and try again. } - if (newState == (int)State.RunningWithPendingCancellation) + if (newState == State.RunningWithPendingCancellation) { // TryComplete will either succeed, or it will see the pending cancellation and deal with it. return false; @@ -243,7 +243,7 @@ public void ProcessCancellation() { Trace("Enter"); - Debug.Assert(_state == (int)State.Canceled); + Debug.Assert(_state == State.Canceled); ErrorCode = SocketError.OperationAborted; @@ -255,7 +255,7 @@ public void ProcessCancellation() else { #if DEBUG - Debug.Assert(Interlocked.CompareExchange(ref _callbackQueued, 1, 0) == 0, $"Unexpected _callbackQueued: {_callbackQueued}"); + Debug.Assert(!Interlocked.Exchange(ref _callbackQueued, true), $"Unexpected _callbackQueued: {_callbackQueued}"); #endif // We've marked the operation as canceled, and so should invoke the callback, but // we can't pool the object, as ProcessQueue may still have a reference to it, due to diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs index f14c6753e93d78..43364203118470 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEngine.Unix.cs @@ -103,7 +103,7 @@ private enum EventQueueProcessingStage Scheduled } - private int _eventQueueProcessingStage; + private EventQueueProcessingStage _eventQueueProcessingStage; // // Registers the Socket with a SocketAsyncEngine, and returns the associated engine. @@ -206,7 +206,7 @@ private void EventLoop() if (handler.HandleSocketEvents(numEvents) && Interlocked.Exchange( ref _eventQueueProcessingStage, - (int)EventQueueProcessingStage.Scheduled) == (int)EventQueueProcessingStage.NotScheduled) + EventQueueProcessingStage.Scheduled) == EventQueueProcessingStage.NotScheduled) { ThreadPool.UnsafeQueueUserWorkItem(this, preferLocal: false); } @@ -223,20 +223,20 @@ private void UpdateEventQueueProcessingStage(bool isEventQueueEmpty) if (!isEventQueueEmpty) { // There are more events to process, set stage to Scheduled and enqueue a work item. - _eventQueueProcessingStage = (int)EventQueueProcessingStage.Scheduled; + _eventQueueProcessingStage = EventQueueProcessingStage.Scheduled; } else { // The stage here would be Scheduled if an enqueuer has enqueued work and changed the stage, or Determining // otherwise. If the stage is Determining, there's no more work to do. If the stage is Scheduled, the enqueuer // would not have scheduled a work item to process the work, so schedule one now. - int stageBeforeUpdate = + EventQueueProcessingStage stageBeforeUpdate = Interlocked.CompareExchange( ref _eventQueueProcessingStage, - (int)EventQueueProcessingStage.NotScheduled, - (int)EventQueueProcessingStage.Determining); - Debug.Assert(stageBeforeUpdate != (int)EventQueueProcessingStage.NotScheduled); - if (stageBeforeUpdate == (int)EventQueueProcessingStage.Determining) + EventQueueProcessingStage.NotScheduled, + EventQueueProcessingStage.Determining); + Debug.Assert(stageBeforeUpdate != EventQueueProcessingStage.NotScheduled); + if (stageBeforeUpdate == EventQueueProcessingStage.Determining) { return; } @@ -251,14 +251,14 @@ void IThreadPoolWorkItem.Execute() SocketIOEvent ev; while (true) { - Debug.Assert(_eventQueueProcessingStage == (int)EventQueueProcessingStage.Scheduled); + Debug.Assert(_eventQueueProcessingStage == EventQueueProcessingStage.Scheduled); // The change needs to be visible to other threads that may request a worker thread before a work item is attempted // to be dequeued by the current thread. In particular, if an enqueuer queues a work item and does not request a // thread because it sees a Determining or Scheduled stage, and the current thread is the last thread processing // work items, the current thread must either see the work item queued by the enqueuer, or it must see a stage of // Scheduled, and try to dequeue again or request another thread. - _eventQueueProcessingStage = (int)EventQueueProcessingStage.Determining; + _eventQueueProcessingStage = EventQueueProcessingStage.Determining; Interlocked.MemoryBarrier(); if (eventQueue.TryDequeue(out ev)) @@ -269,13 +269,13 @@ void IThreadPoolWorkItem.Execute() // The stage here would be Scheduled if an enqueuer has enqueued work and changed the stage, or Determining // otherwise. If the stage is Determining, there's no more work to do. If the stage is Scheduled, the enqueuer // would not have scheduled a work item to process the work, so try to dequeue a work item again. - int stageBeforeUpdate = + EventQueueProcessingStage stageBeforeUpdate = Interlocked.CompareExchange( ref _eventQueueProcessingStage, - (int)EventQueueProcessingStage.NotScheduled, - (int)EventQueueProcessingStage.Determining); - Debug.Assert(stageBeforeUpdate != (int)EventQueueProcessingStage.NotScheduled); - if (stageBeforeUpdate == (int)EventQueueProcessingStage.Determining) + EventQueueProcessingStage.NotScheduled, + EventQueueProcessingStage.Determining); + Debug.Assert(stageBeforeUpdate != EventQueueProcessingStage.NotScheduled); + if (stageBeforeUpdate == EventQueueProcessingStage.Determining) { return; } diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs index 19aa0fa8ab4d75..c98d0cede114e5 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.Windows.cs @@ -100,7 +100,7 @@ private void FreeInternals() private unsafe NativeOverlapped* AllocateNativeOverlapped() { Debug.Assert(OperatingSystem.IsWindows()); - Debug.Assert(_operating == InProgress, $"Expected {nameof(_operating)} == {nameof(InProgress)}, got {_operating}"); + Debug.Assert(_operating == OperationState.InProgress, $"Expected {nameof(_operating)} == {nameof(OperationState.InProgress)}, got {_operating}"); Debug.Assert(_currentSocket != null, "_currentSocket is null"); Debug.Assert(_currentSocket.SafeHandle != null, "_currentSocket.SafeHandle is null"); Debug.Assert(_preAllocatedOverlapped != null, "_preAllocatedOverlapped is null"); @@ -113,7 +113,7 @@ private unsafe void FreeNativeOverlapped(ref NativeOverlapped* overlapped) { Debug.Assert(OperatingSystem.IsWindows()); Debug.Assert(overlapped != null, "overlapped is null"); - Debug.Assert(_operating == InProgress, $"Expected _operating == InProgress, got {_operating}"); + Debug.Assert(_operating == OperationState.InProgress, $"Expected _operating == OperationState.InProgress, got {_operating}"); Debug.Assert(_currentSocket != null, "_currentSocket is null"); Debug.Assert(_currentSocket.SafeHandle != null, "_currentSocket.SafeHandle is null"); Debug.Assert(_currentSocket.SafeHandle.IOCPBoundHandle != null, "_currentSocket.SafeHandle.IOCPBoundHandle is null"); diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs index 78dd22e5eda7bf..dc9fb1d50c4103 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/SocketAsyncEventArgs.cs @@ -75,12 +75,14 @@ public partial class SocketAsyncEventArgs : EventArgs, IDisposable private bool _userSocket; // if false when performing Connect, _currentSocket should be disposed private bool _disposeCalled; - // Controls thread safety via Interlocked. - private const int Configuring = -1; - private const int Free = 0; - private const int InProgress = 1; - private const int Disposed = 2; - private int _operating; + private enum OperationState + { + Configuring = -1, + Free = 0, + InProgress = 1, + Disposed = 2, + } + private OperationState _operating; private CancellationTokenSource? _multipleConnectCancellation; @@ -451,7 +453,7 @@ internal void Complete() _context = null; // Mark as not in-use. - _operating = Free; + _operating = OperationState.Free; // Check for deferred Dispose(). // The deferred Dispose is not guaranteed if Dispose is called while an operation is in progress. @@ -469,7 +471,7 @@ public void Dispose() _disposeCalled = true; // Check if this object is in-use for an async socket operation. - if (Interlocked.CompareExchange(ref _operating, Disposed, Free) != Free) + if (Interlocked.CompareExchange(ref _operating, OperationState.Disposed, OperationState.Free) != OperationState.Free) { // Either already disposed or will be disposed when current operation completes. return; @@ -496,17 +498,17 @@ public void Dispose() // NOTE: Use a try/finally to make sure Complete is called when you're done private void StartConfiguring() { - int status = Interlocked.CompareExchange(ref _operating, Configuring, Free); - if (status != Free) + OperationState status = Interlocked.CompareExchange(ref _operating, OperationState.Configuring, OperationState.Free); + if (status != OperationState.Free) { ThrowForNonFreeStatus(status); } } - private void ThrowForNonFreeStatus(int status) + private void ThrowForNonFreeStatus(OperationState status) { - Debug.Assert(status == InProgress || status == Configuring || status == Disposed, $"Unexpected status: {status}"); - ObjectDisposedException.ThrowIf(status == Disposed, this); + Debug.Assert(status == OperationState.InProgress || status == OperationState.Configuring || status == OperationState.Disposed, $"Unexpected status: {status}"); + ObjectDisposedException.ThrowIf(status == OperationState.Disposed, this); throw new InvalidOperationException(SR.net_socketopinprogress); } @@ -515,8 +517,8 @@ private void ThrowForNonFreeStatus(int status) internal void StartOperationCommon(Socket? socket, SocketAsyncOperation operation) { // Change status to "in-use". - int status = Interlocked.CompareExchange(ref _operating, InProgress, Free); - if (status != Free) + OperationState status = Interlocked.CompareExchange(ref _operating, OperationState.InProgress, OperationState.Free); + if (status != OperationState.Free) { ThrowForNonFreeStatus(status); } @@ -577,7 +579,7 @@ internal void StartOperationConnect(bool saeaMultiConnectCancelable, bool userSo internal void CancelConnectAsync() { - if (_operating == InProgress && _completedOperation == SocketAsyncOperation.Connect) + if (_operating == OperationState.InProgress && _completedOperation == SocketAsyncOperation.Connect) { CancellationTokenSource? multipleConnectCancellation = _multipleConnectCancellation; if (multipleConnectCancellation != null) @@ -836,7 +838,7 @@ caughtException is OperationCanceledException || private sealed class MultiConnectSocketAsyncEventArgs : SocketAsyncEventArgs, IValueTaskSource { private ManualResetValueTaskSourceCore _mrvtsc; - private int _isCompleted; + private bool _isCompleted; public MultiConnectSocketAsyncEventArgs() : base(unsafeSuppressExecutionContextFlow: false) { } @@ -849,7 +851,7 @@ public MultiConnectSocketAsyncEventArgs() : base(unsafeSuppressExecutionContextF protected override void OnCompleted(SocketAsyncEventArgs e) => _mrvtsc.SetResult(true); - public bool ReachedCoordinationPointFirst() => Interlocked.Exchange(ref _isCompleted, 1) == 0; + public bool ReachedCoordinationPointFirst() => !Interlocked.Exchange(ref _isCompleted, true); } internal void FinishOperationSyncSuccess(int bytesTransferred, SocketFlags flags) diff --git a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs index 4d60b197f7a932..54998d3e02e93e 100644 --- a/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs +++ b/src/libraries/System.Net.Sockets/src/System/Net/Sockets/TCPClient.cs @@ -17,10 +17,10 @@ public class TcpClient : IDisposable private AddressFamily _family; private Socket _clientSocket = null!; // initialized by helper called from ctor private NetworkStream? _dataStream; - private volatile int _disposed; + private volatile bool _disposed; private bool _active; - private bool Disposed => _disposed != 0; + private bool Disposed => _disposed; // Initializes a new instance of the System.Net.Sockets.TcpClient class. public TcpClient() : this(AddressFamily.Unknown) @@ -252,7 +252,7 @@ public NetworkStream GetStream() // Disposes the Tcp connection. protected virtual void Dispose(bool disposing) { - if (Interlocked.CompareExchange(ref _disposed, 1, 0) == 0) + if (!Interlocked.Exchange(ref _disposed, true)) { if (disposing) { diff --git a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.NonBrowser.cs b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.NonBrowser.cs index 8716439eac8e25..b865732117622d 100644 --- a/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.NonBrowser.cs +++ b/src/libraries/System.Net.WebProxy/src/System/Net/WebProxy.NonBrowser.cs @@ -11,7 +11,7 @@ public partial class WebProxy : IWebProxy, ISerializable { private static volatile string? s_domainName; private static volatile IPAddress[]? s_localAddresses; - private static int s_networkChangeRegistered; + private static bool s_networkChangeRegistered; private static bool IsLocal(Uri host) { @@ -47,13 +47,13 @@ private static bool IsLocal(Uri host) /// Ensures we've registered with NetworkChange to clear out statically-cached state upon a network change notification. private static void EnsureNetworkChangeRegistration() { - if (s_networkChangeRegistered == 0) + if (!s_networkChangeRegistered) { Register(); static void Register() { - if (Interlocked.Exchange(ref s_networkChangeRegistered, 1) != 0) + if (Interlocked.Exchange(ref s_networkChangeRegistered, true)) { return; } diff --git a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs index 5780555470c1e9..37b5565b74675e 100644 --- a/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs +++ b/src/libraries/System.Net.WebSockets.Client/src/System/Net/WebSockets/ClientWebSocket.cs @@ -12,12 +12,12 @@ namespace System.Net.WebSockets public sealed partial class ClientWebSocket : WebSocket { /// This is really an InternalState value, but Interlocked doesn't support operations on values of enum types. - private int _state; + private InternalState _state; private WebSocketHandle? _innerWebSocket; public ClientWebSocket() { - _state = (int)InternalState.Created; + _state = InternalState.Created; Options = WebSocketHandle.CreateDefaultOptions(); } @@ -39,14 +39,14 @@ public override WebSocketState State return _innerWebSocket.State; } - switch ((InternalState)_state) + switch (_state) { case InternalState.Created: return WebSocketState.None; case InternalState.Connecting: return WebSocketState.Connecting; default: // We only get here if disposed before connecting - Debug.Assert((InternalState)_state == InternalState.Disposed); + Debug.Assert(_state == InternalState.Disposed); return WebSocketState.Closed; } } @@ -105,7 +105,7 @@ public Task ConnectAsync(Uri uri, HttpMessageInvoker? invoker, CancellationToken } // Check that we have not started already. - switch ((InternalState)Interlocked.CompareExchange(ref _state, (int)InternalState.Connecting, (int)InternalState.Created)) + switch (Interlocked.CompareExchange(ref _state, InternalState.Connecting, InternalState.Created)) { case InternalState.Disposed: throw new ObjectDisposedException(GetType().FullName); @@ -135,9 +135,9 @@ private async Task ConnectAsyncCore(Uri uri, HttpMessageInvoker? invoker, Cancel throw; } - if ((InternalState)Interlocked.CompareExchange(ref _state, (int)InternalState.Connected, (int)InternalState.Connecting) != InternalState.Connecting) + if (Interlocked.CompareExchange(ref _state, InternalState.Connected, InternalState.Connecting) != InternalState.Connecting) { - Debug.Assert(_state == (int)InternalState.Disposed); + Debug.Assert(_state == InternalState.Disposed); throw new ObjectDisposedException(GetType().FullName); } } @@ -167,9 +167,9 @@ private WebSocket ConnectedWebSocket { get { - ObjectDisposedException.ThrowIf((InternalState)_state == InternalState.Disposed, this); + ObjectDisposedException.ThrowIf(_state == InternalState.Disposed, this); - if ((InternalState)_state != InternalState.Connected) + if (_state != InternalState.Connected) { throw new InvalidOperationException(SR.net_WebSockets_NotConnected); } @@ -183,7 +183,7 @@ private WebSocket ConnectedWebSocket public override void Abort() { - if ((InternalState)_state != InternalState.Disposed) + if (_state != InternalState.Disposed) { _innerWebSocket?.Abort(); Dispose(); @@ -192,7 +192,7 @@ public override void Abort() public override void Dispose() { - if ((InternalState)Interlocked.Exchange(ref _state, (int)InternalState.Disposed) != InternalState.Disposed) + if (Interlocked.Exchange(ref _state, InternalState.Disposed) != InternalState.Disposed) { _innerWebSocket?.Dispose(); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs b/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs index 3fa044d074d607..8c6ffe9b61c463 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Buffers/SharedArrayPool.cs @@ -35,7 +35,7 @@ internal sealed partial class SharedArrayPool : ArrayPool /// private readonly SharedArrayPoolPartitions?[] _buckets = new SharedArrayPoolPartitions[NumBuckets]; /// Whether the callback to trim arrays in response to memory pressure has been created. - private int _trimCallbackCreated; + private bool _trimCallbackCreated; /// Allocate a new and try to store it into the array. private unsafe SharedArrayPoolPartitions CreatePerCorePartitions(int bucketIndex) @@ -283,7 +283,7 @@ private SharedArrayPoolThreadLocalArray[] InitializeTlsBucketsAndTrimming() t_tlsBuckets = tlsBuckets; _allTlsBuckets.Add(tlsBuckets, null); - if (Interlocked.Exchange(ref _trimCallbackCreated, 1) == 0) + if (!Interlocked.Exchange(ref _trimCallbackCreated, true)) { Gen2GcCallback.Register(s => ((SharedArrayPool)s).Trim(), this); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ActivityTracker.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ActivityTracker.cs index 6bba8725c848ab..8700c8bd52c53b 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ActivityTracker.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ActivityTracker.cs @@ -165,7 +165,7 @@ public void OnStop(string providerName, string activityName, int task, ref Guid ActivityInfo? orphan = currentActivity; while (orphan != activityToStop && orphan != null) { - if (orphan.m_stopped != 0) // Skip dead activities. + if (orphan.m_stopped) // Skip dead activities. { orphan = orphan.m_creator; continue; @@ -177,14 +177,13 @@ public void OnStop(string providerName, string activityName, int task, ref Guid } else { - orphan.m_stopped = 1; - Debug.Assert(orphan.m_stopped != 0); + orphan.m_stopped = true; } orphan = orphan.m_creator; } // try to Stop the activity atomically. Other threads may be trying to do this as well. - if (Interlocked.CompareExchange(ref activityToStop.m_stopped, 1, 0) == 0) + if (!Interlocked.Exchange(ref activityToStop.m_stopped, true)) { // I succeeded stopping this activity. Now we update our m_current pointer @@ -239,7 +238,7 @@ public void Enable() ActivityInfo? activity = startLocation; while (activity != null) { - if (name == activity.m_name && activity.m_stopped == 0) + if (name == activity.m_name && !activity.m_stopped) return activity; activity = activity.m_creator; } @@ -309,7 +308,7 @@ public static string Path(ActivityInfo? activityInfo) public override string ToString() { - return m_name + "(" + Path(this) + (m_stopped != 0 ? ",DEAD)" : ")"); + return m_name + "(" + Path(this) + (m_stopped ? ",DEAD)" : ")"); } public static string LiveActivities(ActivityInfo? list) @@ -520,7 +519,7 @@ private static unsafe void WriteNibble(ref byte* ptr, byte* endPtr, uint value) internal readonly int m_level; // current depth of the Path() of the activity (used to keep recursion under control) internal readonly EventActivityOptions m_eventOptions; // Options passed to start. internal long m_lastChildID; // used to create a unique ID for my children activities - internal int m_stopped; // This work item has stopped + internal bool m_stopped; // This work item has stopped internal readonly ActivityInfo? m_creator; // My parent (creator). Forms the Path() for the activity. internal readonly Guid m_activityIdToRestore; // The Guid to restore after a stop. #endregion @@ -578,7 +577,7 @@ private void ActivityChanging(AsyncLocalValueChangedArgs args) while (cur != null) { // We found a live activity (typically the first time), set it to that. - if (cur.m_stopped == 0) + if (!cur.m_stopped) { EventSource.SetCurrentThreadActivityId(cur.ActivityId); return; diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/CancellationTokenSource.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/CancellationTokenSource.cs index f1358083a220fc..26c2eb70785980 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/CancellationTokenSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/CancellationTokenSource.cs @@ -25,7 +25,7 @@ namespace System.Threading public class CancellationTokenSource : IDisposable { /// A that's already canceled. - internal static readonly CancellationTokenSource s_canceledSource = new CancellationTokenSource() { _state = NotifyingCompleteState }; + internal static readonly CancellationTokenSource s_canceledSource = new CancellationTokenSource() { _state = States.NotifyingCompleteState }; /// A that's never canceled. This isn't enforced programmatically, only by usage. Do not cancel! internal static readonly CancellationTokenSource s_neverCanceledSource = new CancellationTokenSource(); @@ -35,7 +35,7 @@ private static void TimerCallback(object? state) => // separated out into a name ((CancellationTokenSource)state!).NotifyCancellation(throwOnFirstException: false); // skip ThrowIfDisposed() check in Cancel() /// The current state of the CancellationTokenSource. - private volatile int _state; + private volatile States _state; /// Whether this has been disposed. private bool _disposed; /// ITimer used by CancelAfter and Timer-related ctors. Used instead of Timer to avoid extra allocations and because the rooted behavior is desired. @@ -46,10 +46,13 @@ private static void TimerCallback(object? state) => // separated out into a name /// Lazily-initialized, also serving as the lock to protect its contained state. private Registrations? _registrations; - // legal values for _state - private const int NotCanceledState = 0; // default value of _state - private const int NotifyingState = 1; - private const int NotifyingCompleteState = 2; + /// Legal values for . + private enum States + { + NotCanceledState = 0, // default value of _state + NotifyingState = 1, + NotifyingCompleteState = 2, + } /// Gets whether cancellation has been requested for this . /// Whether cancellation has been requested for this . @@ -66,10 +69,10 @@ private static void TimerCallback(object? state) => // separated out into a name /// canceled concurrently. /// /// - public bool IsCancellationRequested => _state != NotCanceledState; + public bool IsCancellationRequested => _state != States.NotCanceledState; /// A simple helper to determine whether cancellation has finished. - internal bool IsCancellationCompleted => _state == NotifyingCompleteState; + internal bool IsCancellationCompleted => _state == States.NotifyingCompleteState; /// Gets the associated with this . /// The associated with this . @@ -201,7 +204,7 @@ private void InitializeWithTimer(TimeSpan millisecondsDelay, TimeProvider timePr { if (millisecondsDelay == TimeSpan.Zero) { - _state = NotifyingCompleteState; + _state = States.NotifyingCompleteState; } else { @@ -480,7 +483,7 @@ public bool TryReset() // We can only reset if cancellation has not yet been requested: we never want to allow a CancellationToken // to transition from canceled to non-canceled. - if (_state == NotCanceledState) + if (_state == States.NotCanceledState) { // If there is no timer, then we're free to reset. If there is a timer, then we need to first try // to reset it to be infinite so that it won't fire, and then recognize that it could have already @@ -556,7 +559,7 @@ protected virtual void Dispose(bool disposing) if (_kernelEvent != null) { ManualResetEvent? mre = Interlocked.Exchange(ref _kernelEvent!, null); - if (mre != null && _state != NotifyingState) + if (mre != null && _state != States.NotifyingState) { mre.Dispose(); } @@ -698,13 +701,13 @@ private void NotifyCancellation(bool throwOnFirstException) } } - /// Transitions from to . + /// Transitions from to . /// true if it successfully transitioned; otherwise, false. /// If it successfully transitions, it will also have disposed of and set . private bool TransitionToCancellationRequested() { if (!IsCancellationRequested && - Interlocked.CompareExchange(ref _state, NotifyingState, NotCanceledState) == NotCanceledState) + Interlocked.CompareExchange(ref _state, States.NotifyingState, States.NotCanceledState) == States.NotCanceledState) { // Dispose of the timer, if any. Dispose may be running concurrently here, but ITimer.Dispose is thread-safe. ITimer? timer = _timer; @@ -737,7 +740,7 @@ private void ExecuteCallbackHandlers(bool throwOnFirstException) Registrations? registrations = Interlocked.Exchange(ref _registrations, null); if (registrations is null) { - Interlocked.Exchange(ref _state, NotifyingCompleteState); + Interlocked.Exchange(ref _state, States.NotifyingCompleteState); return; } @@ -825,7 +828,7 @@ private void ExecuteCallbackHandlers(bool throwOnFirstException) } finally { - _state = NotifyingCompleteState; + _state = States.NotifyingCompleteState; Interlocked.Exchange(ref registrations.ExecutingCallbackId, 0); // for safety, prevent reorderings crossing this point and seeing inconsistent state. } @@ -1021,7 +1024,7 @@ internal sealed class Registrations /// public volatile int ThreadIDExecutingCallbacks = -1; /// Spin lock that protects state in the instance. - private int _lock; + private volatile bool _locked; /// Initializes the instance. /// The associated source. @@ -1030,7 +1033,7 @@ internal sealed class Registrations [MethodImpl(MethodImplOptions.AggressiveInlining)] // used in only two places, one of which is a hot path private void Recycle(CallbackNode node) { - Debug.Assert(_lock == 1); + Debug.Assert(_locked); // Clear out the unused node and put it on the singly-linked free list. // The only field we don't clear out is the associated Registrations, as that's fixed @@ -1163,16 +1166,16 @@ public async ValueTask WaitForCallbackToCompleteAsync(long id) /// Enters the lock for this instance. The current thread must not be holding the lock, but that is not validated. public void EnterLock() { - ref int value = ref _lock; - if (Interlocked.Exchange(ref value, 1) != 0) + ref bool value = ref _locked; + if (Interlocked.Exchange(ref value, true)) { Contention(ref value); [MethodImpl(MethodImplOptions.NoInlining)] - static void Contention(ref int value) + static void Contention(ref bool value) { SpinWait sw = default; - do { sw.SpinOnce(); } while (Interlocked.Exchange(ref value, 1) == 1); + do { sw.SpinOnce(); } while (Interlocked.Exchange(ref value, true)); } } } @@ -1180,8 +1183,8 @@ static void Contention(ref int value) /// Exits the lock for this instance. The current thread must be holding the lock, but that is not validated. public void ExitLock() { - Debug.Assert(_lock == 1); - Volatile.Write(ref _lock, 0); + Debug.Assert(_locked); + _locked = false; } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ReaderWriterLockSlim.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ReaderWriterLockSlim.cs index ed62bbbcc9a68a..222cf6ecd50d01 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ReaderWriterLockSlim.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ReaderWriterLockSlim.cs @@ -1416,7 +1416,7 @@ public int RecursiveWriteCount private struct SpinLock { - private int _isLocked; + private bool _isLocked; /// /// Used to deprioritize threads attempting to enter the lock when they would not make progress after doing so. @@ -1535,7 +1535,7 @@ private bool IsEnterDeprioritized(EnterSpinLockReason reason) [MethodImpl(MethodImplOptions.AggressiveInlining)] private bool TryEnter() { - return Interlocked.CompareExchange(ref _isLocked, 1, 0) == 0; + return !Interlocked.Exchange(ref _isLocked, true); } [MethodImpl(MethodImplOptions.AggressiveInlining)] @@ -1577,7 +1577,7 @@ private void EnterSpin(EnterSpinLockReason reason) if (!IsEnterDeprioritized(reason)) { - if (_isLocked == 0 && TryEnter()) + if (!_isLocked && TryEnter()) { if (deprioritizationStateChange != 0) { @@ -1606,12 +1606,12 @@ private void EnterSpin(EnterSpinLockReason reason) public void Exit() { - Debug.Assert(_isLocked != 0, "Exiting spin lock that is not held"); - Volatile.Write(ref _isLocked, 0); + Debug.Assert(_isLocked, "Exiting spin lock that is not held"); + Volatile.Write(ref _isLocked, false); } #if DEBUG - public bool IsHeld => _isLocked != 0; + public bool IsHeld => _isLocked; #endif } diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs index 365a68c4489acd..98ac5dfe25341d 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPoolWorkQueue.cs @@ -388,7 +388,7 @@ public int Count private bool _loggingEnabled; private bool _dispatchNormalPriorityWorkFirst; - private int _mayHaveHighPriorityWorkItems; + private bool _mayHaveHighPriorityWorkItems; // SOS's ThreadPool command depends on the following names internal readonly ConcurrentQueue workItems = new ConcurrentQueue(); @@ -427,7 +427,7 @@ private struct CacheLineSeparated { private readonly Internal.PaddingFor32 pad1; - public int queueProcessingStage; + public QueueProcessingStage queueProcessingStage; private readonly Internal.PaddingFor32 pad2; } @@ -595,7 +595,7 @@ internal void EnsureThreadRequested() // Otherwise let the current requested thread handle parallelization. if (Interlocked.Exchange( ref _separated.queueProcessingStage, - (int)QueueProcessingStage.Scheduled) == (int)QueueProcessingStage.NotScheduled) + QueueProcessingStage.Scheduled) == QueueProcessingStage.NotScheduled) { ThreadPool.RequestWorkerThread(); } @@ -635,7 +635,7 @@ public void EnqueueAtHighPriority(object workItem) highPriorityWorkItems.Enqueue(workItem); // If the change below is seen by another thread, ensure that the enqueued work item will also be visible - Volatile.Write(ref _mayHaveHighPriorityWorkItems, 1); + Volatile.Write(ref _mayHaveHighPriorityWorkItems, true); EnsureThreadRequested(); } @@ -675,8 +675,8 @@ internal static bool LocalFindAndPop(object callback) tl.isProcessingHighPriorityWorkItems = false; } else if ( - _mayHaveHighPriorityWorkItems != 0 && - Interlocked.CompareExchange(ref _mayHaveHighPriorityWorkItems, 0, 1) != 0 && + _mayHaveHighPriorityWorkItems && + Interlocked.CompareExchange(ref _mayHaveHighPriorityWorkItems, false, true) && TryStartProcessingHighPriorityWorkItemsAndDequeue(tl, out workItem)) { return workItem; @@ -747,7 +747,7 @@ private bool TryStartProcessingHighPriorityWorkItemsAndDequeue( } tl.isProcessingHighPriorityWorkItems = true; - _mayHaveHighPriorityWorkItems = 1; + _mayHaveHighPriorityWorkItems = true; return true; } @@ -830,8 +830,8 @@ internal static bool Dispatch() // thread because it sees a Determining or Scheduled stage, and the current thread is the last thread processing // work items, the current thread must either see the work item queued by the enqueuer, or it must see a stage of // Scheduled, and try to dequeue again or request another thread. - Debug.Assert(workQueue._separated.queueProcessingStage == (int)QueueProcessingStage.Scheduled); - workQueue._separated.queueProcessingStage = (int)QueueProcessingStage.Determining; + Debug.Assert(workQueue._separated.queueProcessingStage == QueueProcessingStage.Scheduled); + workQueue._separated.queueProcessingStage = QueueProcessingStage.Determining; Interlocked.MemoryBarrier(); object? workItem = null; @@ -859,8 +859,8 @@ internal static bool Dispatch() workQueue.UnassignWorkItemQueue(tl); } - Debug.Assert(workQueue._separated.queueProcessingStage != (int)QueueProcessingStage.NotScheduled); - workQueue._separated.queueProcessingStage = (int)QueueProcessingStage.Scheduled; + Debug.Assert(workQueue._separated.queueProcessingStage != QueueProcessingStage.NotScheduled); + workQueue._separated.queueProcessingStage = QueueProcessingStage.Scheduled; ThreadPool.RequestWorkerThread(); return true; } @@ -868,13 +868,13 @@ internal static bool Dispatch() // The stage here would be Scheduled if an enqueuer has enqueued work and changed the stage, or Determining // otherwise. If the stage is Determining, there's no more work to do. If the stage is Scheduled, the enqueuer // would not have scheduled a work item to process the work, so try to dequeue a work item again. - int stageBeforeUpdate = + QueueProcessingStage stageBeforeUpdate = Interlocked.CompareExchange( ref workQueue._separated.queueProcessingStage, - (int)QueueProcessingStage.NotScheduled, - (int)QueueProcessingStage.Determining); - Debug.Assert(stageBeforeUpdate != (int)QueueProcessingStage.NotScheduled); - if (stageBeforeUpdate == (int)QueueProcessingStage.Determining) + QueueProcessingStage.NotScheduled, + QueueProcessingStage.Determining); + Debug.Assert(stageBeforeUpdate != QueueProcessingStage.NotScheduled); + if (stageBeforeUpdate == QueueProcessingStage.Determining) { if (s_assignableWorkItemQueueCount > 0) { @@ -888,7 +888,7 @@ internal static bool Dispatch() // by the enqueuer. Set the stage back to Determining and try to dequeue a work item again. // // See the first similarly used memory barrier in the method for why it's necessary. - workQueue._separated.queueProcessingStage = (int)QueueProcessingStage.Determining; + workQueue._separated.queueProcessingStage = QueueProcessingStage.Determining; Interlocked.MemoryBarrier(); } } @@ -900,7 +900,7 @@ internal static bool Dispatch() // be able to detect if an enqueue races with the dequeue below. // // See the first similarly used memory barrier in the method for why it's necessary. - workQueue._separated.queueProcessingStage = (int)QueueProcessingStage.Determining; + workQueue._separated.queueProcessingStage = QueueProcessingStage.Determining; Interlocked.MemoryBarrier(); object? secondWorkItem = DequeueWithPriorityAlternation(workQueue, tl, out bool missedSteal); @@ -917,8 +917,8 @@ internal static bool Dispatch() // responsibility of the new thread and other enqueuers to request more threads as necessary. The // parallelization may be necessary here for correctness (aside from perf) if the work item blocks for some // reason that may have a dependency on other queued work items. - Debug.Assert(workQueue._separated.queueProcessingStage != (int)QueueProcessingStage.NotScheduled); - workQueue._separated.queueProcessingStage = (int)QueueProcessingStage.Scheduled; + Debug.Assert(workQueue._separated.queueProcessingStage != QueueProcessingStage.NotScheduled); + workQueue._separated.queueProcessingStage = QueueProcessingStage.Scheduled; ThreadPool.RequestWorkerThread(); } else @@ -926,13 +926,13 @@ internal static bool Dispatch() // The stage here would be Scheduled if an enqueuer has enqueued work and changed the stage, or Determining // otherwise. If the stage is Determining, there's no more work to do. If the stage is Scheduled, the enqueuer // would not have requested a thread, so request one now. - int stageBeforeUpdate = + QueueProcessingStage stageBeforeUpdate = Interlocked.CompareExchange( ref workQueue._separated.queueProcessingStage, - (int)QueueProcessingStage.NotScheduled, - (int)QueueProcessingStage.Determining); - Debug.Assert(stageBeforeUpdate != (int)QueueProcessingStage.NotScheduled); - if (stageBeforeUpdate == (int)QueueProcessingStage.Scheduled) + QueueProcessingStage.NotScheduled, + QueueProcessingStage.Determining); + Debug.Assert(stageBeforeUpdate != QueueProcessingStage.NotScheduled); + if (stageBeforeUpdate == QueueProcessingStage.Scheduled) { // A work item was enqueued after the stage was set to Determining earlier, and a thread was not // requested by the enqueuer, so request a thread now. An alternate is to retry dequeuing, as requesting @@ -1193,7 +1193,7 @@ private enum QueueProcessingStage Scheduled } - private int _queueProcessingStage; + private QueueProcessingStage _queueProcessingStage; private readonly ConcurrentQueue _workItems = new ConcurrentQueue(); public int Count => _workItems.Count; @@ -1211,7 +1211,7 @@ public void CompleteBatchEnqueue() // Otherwise there must be a work item already queued or another thread already handling parallelization. if (Interlocked.Exchange( ref _queueProcessingStage, - (int)QueueProcessingStage.Scheduled) == (int)QueueProcessingStage.NotScheduled) + QueueProcessingStage.Scheduled) == QueueProcessingStage.NotScheduled) { ThreadPool.UnsafeQueueHighPriorityWorkItemInternal(this); } @@ -1222,20 +1222,20 @@ private void UpdateQueueProcessingStage(bool isQueueEmpty) if (!isQueueEmpty) { // There are more items to process, set stage to Scheduled and enqueue a TP work item. - _queueProcessingStage = (int)QueueProcessingStage.Scheduled; + _queueProcessingStage = QueueProcessingStage.Scheduled; } else { // The stage here would be Scheduled if an enqueuer has enqueued work and changed the stage, or Determining // otherwise. If the stage is Determining, there's no more work to do. If the stage is Scheduled, the enqueuer // would not have scheduled a work item to process the work, so schedule one one. - int stageBeforeUpdate = + QueueProcessingStage stageBeforeUpdate = Interlocked.CompareExchange( ref _queueProcessingStage, - (int)QueueProcessingStage.NotScheduled, - (int)QueueProcessingStage.Determining); - Debug.Assert(stageBeforeUpdate != (int)QueueProcessingStage.NotScheduled); - if (stageBeforeUpdate == (int)QueueProcessingStage.Determining) + QueueProcessingStage.NotScheduled, + QueueProcessingStage.Determining); + Debug.Assert(stageBeforeUpdate != QueueProcessingStage.NotScheduled); + if (stageBeforeUpdate == QueueProcessingStage.Determining) { return; } @@ -1249,14 +1249,14 @@ void IThreadPoolWorkItem.Execute() T workItem; while (true) { - Debug.Assert(_queueProcessingStage == (int)QueueProcessingStage.Scheduled); + Debug.Assert(_queueProcessingStage == QueueProcessingStage.Scheduled); // The change needs to be visible to other threads that may request a worker thread before a work item is attempted // to be dequeued by the current thread. In particular, if an enqueuer queues a work item and does not request a // thread because it sees a Determining or Scheduled stage, and the current thread is the last thread processing // work items, the current thread must either see the work item queued by the enqueuer, or it must see a stage of // Scheduled, and try to dequeue again or request another thread. - _queueProcessingStage = (int)QueueProcessingStage.Determining; + _queueProcessingStage = QueueProcessingStage.Determining; Interlocked.MemoryBarrier(); if (_workItems.TryDequeue(out workItem)) @@ -1267,13 +1267,13 @@ void IThreadPoolWorkItem.Execute() // The stage here would be Scheduled if an enqueuer has enqueued work and changed the stage, or Determining // otherwise. If the stage is Determining, there's no more work to do. If the stage is Scheduled, the enqueuer // would not have scheduled a work item to process the work, so try to dequeue a work item again. - int stageBeforeUpdate = + QueueProcessingStage stageBeforeUpdate = Interlocked.CompareExchange( ref _queueProcessingStage, - (int)QueueProcessingStage.NotScheduled, - (int)QueueProcessingStage.Determining); - Debug.Assert(stageBeforeUpdate != (int)QueueProcessingStage.NotScheduled); - if (stageBeforeUpdate == (int)QueueProcessingStage.Determining) + QueueProcessingStage.NotScheduled, + QueueProcessingStage.Determining); + Debug.Assert(stageBeforeUpdate != QueueProcessingStage.NotScheduled); + if (stageBeforeUpdate == QueueProcessingStage.Determining) { return; } @@ -1325,13 +1325,12 @@ void IThreadPoolWorkItem.Execute() internal abstract class QueueUserWorkItemCallbackBase : IThreadPoolWorkItem { #if DEBUG - private int executed; + private bool _executed; ~QueueUserWorkItemCallbackBase() { Interlocked.MemoryBarrier(); // ensure that an old cached value is not read below - Debug.Assert( - executed != 0, "A QueueUserWorkItemCallback was never called!"); + Debug.Assert(_executed, "A QueueUserWorkItemCallback was never called!"); } #endif @@ -1339,9 +1338,7 @@ public virtual void Execute() { #if DEBUG GC.SuppressFinalize(this); - Debug.Assert( - 0 == Interlocked.Exchange(ref executed, 1), - "A QueueUserWorkItemCallback was called twice!"); + Debug.Assert(!Interlocked.Exchange(ref _executed, true), "A QueueUserWorkItemCallback was called twice!"); #endif } } diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs index 8bcafa15a6271b..a26587e5412a5f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/WaitSubsystem.ThreadWaitInfo.Unix.cs @@ -75,7 +75,7 @@ public sealed class ThreadWaitInfo /// - Sleep(0) intentionally does not acquire any lock, so it uses an interlocked compare-exchange for the read and /// reset, see /// - private int _isPendingInterrupt; + private bool _isPendingInterrupt; //////////////////////////////////////////////////////////////// @@ -508,7 +508,7 @@ private void RecordPendingInterrupt() s_lock.VerifyIsLocked(); _waitMonitor.VerifyIsLocked(); - _isPendingInterrupt = 1; + _isPendingInterrupt = true; } public bool CheckAndResetPendingInterrupt @@ -519,11 +519,11 @@ public bool CheckAndResetPendingInterrupt Debug.Assert(s_lock.IsLocked || _waitMonitor.IsLocked); #endif - if (_isPendingInterrupt == 0) + if (!_isPendingInterrupt) { return false; } - _isPendingInterrupt = 0; + _isPendingInterrupt = false; return true; } } @@ -535,7 +535,7 @@ private bool CheckAndResetPendingInterrupt_NotLocked s_lock.VerifyIsNotLocked(); _waitMonitor.VerifyIsNotLocked(); - return Interlocked.CompareExchange(ref _isPendingInterrupt, 0, 1) != 0; + return Interlocked.CompareExchange(ref _isPendingInterrupt, false, true); } } diff --git a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/ComObject.cs b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/ComObject.cs index 216b38d7de253f..a88f3edaecff29 100644 --- a/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/ComObject.cs +++ b/src/libraries/System.Runtime.InteropServices/src/System/Runtime/InteropServices/Marshalling/ComObject.cs @@ -23,8 +23,7 @@ public sealed unsafe class ComObject : IDynamicInterfaceCastable, IUnmanagedVirt private readonly object? _runtimeCallableWrapper; - // This is an int so we can use the Interlocked APIs to update it. - private volatile int _released; + private volatile bool _released; /// /// Initialize ComObject instance. @@ -81,7 +80,7 @@ internal ComObject(IIUnknownInterfaceDetailsStrategy interfaceDetailsStrategy, I /// public void FinalRelease() { - if (UniqueInstance && Interlocked.CompareExchange(ref _released, 1, 0) == 0) + if (UniqueInstance && !Interlocked.Exchange(ref _released, true)) { GC.SuppressFinalize(this); CacheStrategy.Clear(IUnknownStrategy); @@ -115,7 +114,7 @@ bool IDynamicInterfaceCastable.IsInterfaceImplemented(RuntimeTypeHandle interfac private bool LookUpVTableInfo(RuntimeTypeHandle handle, out IIUnknownCacheStrategy.TableInfo result, out int qiHResult) { - ObjectDisposedException.ThrowIf(_released != 0, this); + ObjectDisposedException.ThrowIf(_released, this); qiHResult = 0; if (!CacheStrategy.TryGetTableInfo(handle, out result)) diff --git a/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/Parallel.ForEachAsync.cs b/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/Parallel.ForEachAsync.cs index ef700e1b9c967b..643ea8590f0cb0 100644 --- a/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/Parallel.ForEachAsync.cs +++ b/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/Parallel.ForEachAsync.cs @@ -92,34 +92,10 @@ private static Task ForAsync(T fromInclusive, T toExclusive, int dop, TaskSch return Task.CompletedTask; } - [MethodImpl(MethodImplOptions.AggressiveInlining)] - static bool Interlockable() => - typeof(T) == typeof(sbyte) || - typeof(T) == typeof(byte) || - typeof(T) == typeof(short) || - typeof(T) == typeof(ushort) || - typeof(T) == typeof(char) || - typeof(T) == typeof(int) || - typeof(T) == typeof(uint) || - typeof(T) == typeof(long) || - typeof(T) == typeof(ulong) || - typeof(T) == typeof(nint) || - typeof(T) == typeof(nuint); - -#pragma warning disable CS8500 - [MethodImpl(MethodImplOptions.AggressiveInlining)] - static unsafe bool CompareExchange(ref T location, T value, T comparand) => - sizeof(T) == sizeof(byte) ? Interlocked.CompareExchange(ref Unsafe.As(ref location), Unsafe.As(ref value), Unsafe.As(ref comparand)) == Unsafe.As(ref comparand) : - sizeof(T) == sizeof(ushort) ? Interlocked.CompareExchange(ref Unsafe.As(ref location), Unsafe.As(ref value), Unsafe.As(ref comparand)) == Unsafe.As(ref comparand) : - sizeof(T) == sizeof(uint) ? Interlocked.CompareExchange(ref Unsafe.As(ref location), Unsafe.As(ref value), Unsafe.As(ref comparand)) == Unsafe.As(ref comparand) : - sizeof(T) == sizeof(ulong) ? Interlocked.CompareExchange(ref Unsafe.As(ref location), Unsafe.As(ref value), Unsafe.As(ref comparand)) == Unsafe.As(ref comparand) : - throw new UnreachableException(); -#pragma warning restore CS8500 - // The worker body. Each worker will execute this same body. Func taskBody = static async o => { - var state = (ForEachState)o; + var state = (ForAsyncState)o; bool launchedNext = false; #pragma warning disable CA2007 // Explicitly don't use ConfigureAwait, as we want to perform all work on the specified scheduler that's now current @@ -128,10 +104,10 @@ static unsafe bool CompareExchange(ref T location, T value, T comparand) => // Continue to loop while there are more elements to be processed. while (!state.Cancellation.IsCancellationRequested) { - // Get the next element from the enumerator. For some types, we can get the next element with just + // Get the next element from the enumerator. For primitive types, we can get the next element with just // interlocked operations, avoiding the need to take a lock. For other types, we need to take a lock. T element; - if (Interlockable()) + if (typeof(T).IsPrimitive) { TryAgain: element = state.NextAvailable; @@ -140,7 +116,7 @@ static unsafe bool CompareExchange(ref T location, T value, T comparand) => break; } - if (!CompareExchange(ref state.NextAvailable, element + T.One, element)) + if (Interlocked.CompareExchange(ref state.NextAvailable, element + T.One, element) != element) { goto TryAgain; } @@ -202,7 +178,7 @@ static unsafe bool CompareExchange(ref T location, T value, T comparand) => { // Construct a state object that encapsulates all state to be passed and shared between // the workers, and queues the first worker. - var state = new ForEachState(fromInclusive, toExclusive, taskBody, !Interlockable(), dop, scheduler, cancellationToken, body); + var state = new ForAsyncState(fromInclusive, toExclusive, taskBody, dop, scheduler, cancellationToken, body); state.QueueWorkerIfDopAvailable(); return state.Task; } @@ -749,18 +725,18 @@ public ValueTask DisposeAsync() } } - /// Stores the state associated with an IAsyncEnumerable ForEachAsync operation, shared between all its workers. + /// Stores the state associated with an IAsyncEnumerable ForAsyncState operation, shared between all its workers. /// Specifies the type of data being enumerated. - private sealed class ForEachState : ForEachAsyncState, IDisposable + private sealed class ForAsyncState : ForEachAsyncState, IDisposable { public T NextAvailable; public readonly T ToExclusive; - public ForEachState( + public ForAsyncState( T fromExclusive, T toExclusive, Func taskBody, - bool needsLock, int dop, TaskScheduler scheduler, CancellationToken cancellationToken, + int dop, TaskScheduler scheduler, CancellationToken cancellationToken, Func body) : - base(taskBody, needsLock, dop, scheduler, cancellationToken, body) + base(taskBody, !typeof(T).IsPrimitive, dop, scheduler, cancellationToken, body) { NextAvailable = fromExclusive; ToExclusive = toExclusive; diff --git a/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelLoopState.cs b/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelLoopState.cs index e00c096abb97e2..65f0ba68298efa 100644 --- a/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelLoopState.cs +++ b/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelLoopState.cs @@ -211,9 +211,7 @@ internal static void Break(TInt iteration, ParallelLoopStateFlags pf if (iteration < oldLBI) { SpinWait wait = default; - while (typeof(TInt) == typeof(int) ? - Interlocked.CompareExchange(ref Unsafe.As(ref pflags._lowestBreakIteration), Unsafe.As(ref iteration), Unsafe.As(ref oldLBI)) != Unsafe.As(ref oldLBI) : - Interlocked.CompareExchange(ref Unsafe.As(ref pflags._lowestBreakIteration), Unsafe.As(ref iteration), Unsafe.As(ref oldLBI)) != Unsafe.As(ref oldLBI)) + while (Interlocked.CompareExchange(ref pflags._lowestBreakIteration, iteration, oldLBI) != oldLBI) { wait.SpinOnce(); oldLBI = pflags.LowestBreakIteration; diff --git a/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelRangeManager.cs b/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelRangeManager.cs index c53a3346c7d9b7..a1b4d65c6224a6 100644 --- a/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelRangeManager.cs +++ b/src/libraries/System.Threading.Tasks.Parallel/src/System/Threading/Tasks/ParallelRangeManager.cs @@ -33,8 +33,8 @@ internal struct IndexRange // that range, minimizing the chances it'll be near the objects from other threads. internal volatile StrongBox? _nSharedCurrentIndexOffset; - // to be set to 1 by the worker that finishes this range. It's OK to do a non-interlocked write here. - internal int _bRangeFinished; + // to be set to true by the worker that finishes this range. It's OK to do a non-interlocked write here. + internal bool _bRangeFinished; } @@ -101,7 +101,7 @@ private bool FindNewWork(out long nFromInclusiveLocal, out long nToExclusiveLoca // local snap to save array access bounds checks in places where we only read fields IndexRange currentRange = _indexRanges[_nCurrentIndexRange]; - if (currentRange._bRangeFinished == 0) + if (!currentRange._bRangeFinished) { StrongBox? sharedCurrentIndexOffset = _indexRanges[_nCurrentIndexRange]._nSharedCurrentIndexOffset; if (sharedCurrentIndexOffset == null) @@ -157,7 +157,7 @@ private bool FindNewWork(out long nFromInclusiveLocal, out long nToExclusiveLoca else { // this index range is completed, mark it so that others can skip it quickly - Interlocked.Exchange(ref _indexRanges[_nCurrentIndexRange]._bRangeFinished, 1); + Interlocked.Exchange(ref _indexRanges[_nCurrentIndexRange]._bRangeFinished, true); } } @@ -262,7 +262,7 @@ internal RangeManager(long nFromInclusive, long nToExclusive, long nStep, int nN // the fromInclusive of the new index range is always on nCurrentIndex _indexRanges[i]._nFromInclusive = nCurrentIndex; _indexRanges[i]._nSharedCurrentIndexOffset = null; - _indexRanges[i]._bRangeFinished = 0; + _indexRanges[i]._bRangeFinished = false; // now increment it to find the toExclusive value for our range nCurrentIndex = unchecked(nCurrentIndex + nRangeSize); diff --git a/src/libraries/System.Threading.ThreadPool/tests/ThreadPoolTests.cs b/src/libraries/System.Threading.ThreadPool/tests/ThreadPoolTests.cs index c96ad22b47b531..c89bee5faa8891 100644 --- a/src/libraries/System.Threading.ThreadPool/tests/ThreadPoolTests.cs +++ b/src/libraries/System.Threading.ThreadPool/tests/ThreadPoolTests.cs @@ -452,7 +452,7 @@ public void MetricsTest() bool waitForWorkStart = false; var workStarted = new AutoResetEvent(false); var localWorkScheduled = new AutoResetEvent(false); - int completeWork = 0; + bool completeWork = false; int queuedWorkCount = 0; var allWorkCompleted = new ManualResetEvent(false); Exception backgroundEx = null; @@ -467,7 +467,7 @@ public void MetricsTest() // Blocking can affect thread pool thread injection heuristics, so don't block, pretend like a // long-running CPU-bound work item ThreadTestHelpers.WaitForConditionWithoutRelinquishingTimeSlice( - () => Interlocked.CompareExchange(ref completeWork, 0, 0) != 0); + () => Interlocked.CompareExchange(ref completeWork, false, false)); } catch (Exception ex) { @@ -557,7 +557,7 @@ public void MetricsTest() finally { // Complete the work - Interlocked.Exchange(ref completeWork, 1); + Interlocked.Exchange(ref completeWork, true); } // Wait for work items to exit, for counting