Skip to content

Commit 0ad780b

Browse files
stephentoubstephentoub
committed
Use generic Interlocked.{Compare}Exchange in more places
1 parent b0f578c commit 0ad780b

File tree

48 files changed

+415
-422
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+415
-422
lines changed

‎src/coreclr/nativeaot/System.Private.CoreLib/src/System/EventReporter.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ private static unsafe void ClrReportEvent(string eventSource, short type, ushort
171171
Interop.Advapi32.DeregisterEventSource(handle);
172172
}
173173

174-
private static byte s_once;
174+
private static bool s_once;
175175

176176
public static bool ShouldLogInEventLog
177177
{
@@ -180,7 +180,7 @@ public static bool ShouldLogInEventLog
180180
if (Interop.Kernel32.IsDebuggerPresent())
181181
return false;
182182

183-
if (s_once == 1 || Interlocked.Exchange(ref s_once, 1) == 1)
183+
if (s_once || Interlocked.Exchange(ref s_once, true))
184184
return false;
185185

186186
return true;

‎src/libraries/Common/src/System/Net/StreamBuffer.cs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -292,15 +292,15 @@ private sealed class ResettableValueTaskSource : IValueTaskSource
292292

293293
private ManualResetValueTaskSourceCore<bool> _waitSource; // mutable struct, do not make this readonly
294294
private CancellationTokenRegistration _waitSourceCancellation;
295-
private int _hasWaiter;
295+
private bool _hasWaiter;
296296

297297
ValueTaskSourceStatus IValueTaskSource.GetStatus(short token) => _waitSource.GetStatus(token);
298298

299299
void IValueTaskSource.OnCompleted(Action<object?> continuation, object? state, short token, ValueTaskSourceOnCompletedFlags flags) => _waitSource.OnCompleted(continuation, state, token, flags);
300300

301301
void IValueTaskSource.GetResult(short token)
302302
{
303-
Debug.Assert(_hasWaiter == 0);
303+
Debug.Assert(!_hasWaiter);
304304

305305
// Clean up the registration. This will wait for any in-flight cancellation to complete.
306306
_waitSourceCancellation.Dispose();
@@ -312,7 +312,7 @@ void IValueTaskSource.GetResult(short token)
312312

313313
public void SignalWaiter()
314314
{
315-
if (Interlocked.Exchange(ref _hasWaiter, 0) == 1)
315+
if (Interlocked.Exchange(ref _hasWaiter, false))
316316
{
317317
_waitSource.SetResult(true);
318318
}
@@ -322,21 +322,21 @@ private void CancelWaiter(CancellationToken cancellationToken)
322322
{
323323
Debug.Assert(cancellationToken.IsCancellationRequested);
324324

325-
if (Interlocked.Exchange(ref _hasWaiter, 0) == 1)
325+
if (Interlocked.Exchange(ref _hasWaiter, false))
326326
{
327327
_waitSource.SetException(ExceptionDispatchInfo.SetCurrentStackTrace(new OperationCanceledException(cancellationToken)));
328328
}
329329
}
330330

331331
public void Reset()
332332
{
333-
if (_hasWaiter != 0)
333+
if (_hasWaiter)
334334
{
335335
throw new InvalidOperationException("Concurrent use is not supported");
336336
}
337337

338338
_waitSource.Reset();
339-
Volatile.Write(ref _hasWaiter, 1);
339+
Volatile.Write(ref _hasWaiter, true);
340340
}
341341

342342
public void Wait()

‎src/libraries/Common/tests/System/Net/WebSockets/WebSocketStream.cs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,8 @@ public class WebSocketStream : Stream
2121
// Used by the class to indicate that the stream is writable.
2222
private bool _writeable;
2323

24-
// Whether Dispose has been called. 0 == false, 1 == true
25-
private int _disposed;
24+
// Whether Dispose has been called.
25+
private bool _disposed;
2626

2727
public WebSocketStream(WebSocket socket)
2828
: this(socket, FileAccess.ReadWrite, ownsSocket: false)
@@ -140,7 +140,7 @@ public void Close(int timeout)
140140

141141
protected override void Dispose(bool disposing)
142142
{
143-
if (Interlocked.Exchange(ref _disposed, 1) != 0)
143+
if (Interlocked.Exchange(ref _disposed, true))
144144
{
145145
return;
146146
}
@@ -269,7 +269,7 @@ public override void SetLength(long value)
269269

270270
private void ThrowIfDisposed()
271271
{
272-
ObjectDisposedException.ThrowIf(_disposed != 0, this);
272+
ObjectDisposedException.ThrowIf(_disposed, this);
273273
}
274274

275275
private static IOException WrapException(string resourceFormatString, Exception innerException)

‎src/libraries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentBag.cs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -628,11 +628,11 @@ private void FreezeBag(ref bool lockTaken)
628628
// Finally, wait for all unsynchronized operations on each queue to be done.
629629
for (WorkStealingQueue? queue = head; queue != null; queue = queue._nextQueue)
630630
{
631-
if (queue._currentOp != (int)Operation.None)
631+
if (queue._currentOp != Operation.None)
632632
{
633633
SpinWait spinner = default;
634634
do { spinner.SpinOnce(); }
635-
while (queue._currentOp != (int)Operation.None);
635+
while (queue._currentOp != Operation.None);
636636
}
637637
}
638638
}
@@ -684,7 +684,7 @@ private sealed class WorkStealingQueue
684684
/// <summary>Number of steals; needs to be combined with <see cref="_addTakeCount"/> to get an actual Count.</summary>
685685
private int _stealCount;
686686
/// <summary>The current queue operation. Used to quiesce before performing operations from one thread onto another.</summary>
687-
internal volatile int _currentOp;
687+
internal volatile Operation _currentOp;
688688
/// <summary>true if this queue's lock is held as part of a global freeze.</summary>
689689
internal bool _frozen;
690690
/// <summary>Next queue in the <see cref="ConcurrentBag{T}"/>'s set of thread-local queues.</summary>
@@ -726,14 +726,14 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount)
726726
try
727727
{
728728
// Full fence to ensure subsequent reads don't get reordered before this
729-
Interlocked.Exchange(ref _currentOp, (int)Operation.Add);
729+
Interlocked.Exchange(ref _currentOp, Operation.Add);
730730
int tail = _tailIndex;
731731

732732
// Rare corner case (at most once every 2 billion pushes on this thread):
733733
// We're going to increment the tail; if we'll overflow, then we need to reset our counts
734734
if (tail == int.MaxValue)
735735
{
736-
_currentOp = (int)Operation.None; // set back to None temporarily to avoid a deadlock
736+
_currentOp = Operation.None; // set back to None temporarily to avoid a deadlock
737737
lock (this)
738738
{
739739
Debug.Assert(_tailIndex == tail, "No other thread should be changing _tailIndex");
@@ -749,7 +749,7 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount)
749749
_tailIndex = tail &= _mask;
750750
Debug.Assert(_headIndex - _tailIndex <= 0);
751751

752-
Interlocked.Exchange(ref _currentOp, (int)Operation.Add); // ensure subsequent reads aren't reordered before this
752+
Interlocked.Exchange(ref _currentOp, Operation.Add); // ensure subsequent reads aren't reordered before this
753753
}
754754
}
755755

@@ -778,7 +778,7 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount)
778778
else
779779
{
780780
// We need to contend with foreign operations (e.g. steals, enumeration, etc.), so we lock.
781-
_currentOp = (int)Operation.None; // set back to None to avoid a deadlock
781+
_currentOp = Operation.None; // set back to None to avoid a deadlock
782782
Monitor.Enter(this, ref lockTaken);
783783

784784
head = _headIndex;
@@ -830,7 +830,7 @@ internal void LocalPush(T item, ref long emptyToNonEmptyListTransitionCount)
830830
}
831831
finally
832832
{
833-
_currentOp = (int)Operation.None;
833+
_currentOp = Operation.None;
834834
if (lockTaken)
835835
{
836836
Monitor.Exit(this);
@@ -874,7 +874,7 @@ internal bool TryLocalPop([MaybeNullWhen(false)] out T result)
874874
// If the read of _headIndex moved before this write to _tailIndex, we could erroneously end up
875875
// popping an element that's concurrently being stolen, leading to the same element being
876876
// dequeued from the bag twice.
877-
_currentOp = (int)Operation.Take;
877+
_currentOp = Operation.Take;
878878
Interlocked.Exchange(ref _tailIndex, --tail);
879879

880880
// 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)
895895
else
896896
{
897897
// Interaction with steals: 0 or 1 elements left.
898-
_currentOp = (int)Operation.None; // set back to None to avoid a deadlock
898+
_currentOp = Operation.None; // set back to None to avoid a deadlock
899899
Monitor.Enter(this, ref lockTaken);
900900
if (_headIndex - tail <= 0)
901901
{
@@ -920,7 +920,7 @@ internal bool TryLocalPop([MaybeNullWhen(false)] out T result)
920920
}
921921
finally
922922
{
923-
_currentOp = (int)Operation.None;
923+
_currentOp = Operation.None;
924924
if (lockTaken)
925925
{
926926
Monitor.Exit(this);
@@ -975,14 +975,14 @@ internal bool TrySteal([MaybeNullWhen(false)] out T result, bool take)
975975
// is in progress, as add operations need to accurately count transitions
976976
// from empty to non-empty, and they can only do that if there are no concurrent
977977
// steal operations happening at the time.
978-
if ((head - (_tailIndex - 2) >= 0) && _currentOp == (int)Operation.Add)
978+
if ((head - (_tailIndex - 2) >= 0) && _currentOp == Operation.Add)
979979
{
980980
SpinWait spinner = default;
981981
do
982982
{
983983
spinner.SpinOnce();
984984
}
985-
while (_currentOp == (int)Operation.Add);
985+
while (_currentOp == Operation.Add);
986986
}
987987

988988
// Increment head to tentatively take an element: a full fence is used to ensure the read

‎src/libraries/System.ComponentModel.TypeConverter/tests/TypeDescriptorTests.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,11 +1276,11 @@ class TwiceDerivedCultureInfo : DerivedCultureInfo
12761276
{
12771277
}
12781278

1279-
private long _concurrentError = 0;
1279+
private volatile bool _concurrentError;
12801280
private bool ConcurrentError
12811281
{
1282-
get => Interlocked.Read(ref _concurrentError) == 1;
1283-
set => Interlocked.Exchange(ref _concurrentError, value ? 1 : 0);
1282+
get => _concurrentError;
1283+
set => Interlocked.Exchange(ref _concurrentError, value);
12841284
}
12851285

12861286
private void ConcurrentTest(TypeWithProperty instance)

‎src/libraries/System.IO.Compression.Brotli/src/System/IO/Compression/BrotliStream.cs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ private void ReleaseStateForDispose()
128128
if (buffer != null)
129129
{
130130
_buffer = null!;
131-
if (!AsyncOperationIsActive)
131+
if (!_activeAsyncOperation)
132132
{
133133
ArrayPool<byte>.Shared.Return(buffer);
134134
}
@@ -170,27 +170,28 @@ public override long Position
170170
/// <param name="value">The length of the stream.</param>
171171
public override void SetLength(long value) => throw new NotSupportedException();
172172

173-
private int _activeAsyncOperation; // 1 == true, 0 == false
174-
private bool AsyncOperationIsActive => _activeAsyncOperation != 0;
173+
private volatile bool _activeAsyncOperation;
175174

176175
private void EnsureNoActiveAsyncOperation()
177176
{
178-
if (AsyncOperationIsActive)
177+
if (_activeAsyncOperation)
178+
{
179179
ThrowInvalidBeginCall();
180+
}
180181
}
181182

182183
private void AsyncOperationStarting()
183184
{
184-
if (Interlocked.Exchange(ref _activeAsyncOperation, 1) != 0)
185+
if (Interlocked.Exchange(ref _activeAsyncOperation, true))
185186
{
186187
ThrowInvalidBeginCall();
187188
}
188189
}
189190

190191
private void AsyncOperationCompleting()
191192
{
192-
Debug.Assert(_activeAsyncOperation == 1);
193-
Volatile.Write(ref _activeAsyncOperation, 0);
193+
Debug.Assert(_activeAsyncOperation);
194+
_activeAsyncOperation = false;
194195
}
195196

196197
private static void ThrowInvalidBeginCall() =>

‎src/libraries/System.IO.Compression/src/System/IO/Compression/DeflateZLib/DeflateStream.cs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ public partial class DeflateStream : Stream
2020
private Inflater? _inflater;
2121
private Deflater? _deflater;
2222
private byte[]? _buffer;
23-
private int _activeAsyncOperation; // 1 == true, 0 == false
23+
private volatile bool _activeAsyncOperation;
2424
private bool _wroteBytes;
2525

2626
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)
698698
if (buffer != null)
699699
{
700700
_buffer = null;
701-
if (!AsyncOperationIsActive)
701+
if (!_activeAsyncOperation)
702702
{
703703
ArrayPool<byte>.Shared.Return(buffer);
704704
}
@@ -751,7 +751,7 @@ async ValueTask Core()
751751
if (buffer != null)
752752
{
753753
_buffer = null;
754-
if (!AsyncOperationIsActive)
754+
if (!_activeAsyncOperation)
755755
{
756756
ArrayPool<byte>.Shared.Return(buffer);
757757
}
@@ -1059,24 +1059,27 @@ public override void Flush() { }
10591059
public override void SetLength(long value) { throw new NotSupportedException(); }
10601060
}
10611061

1062-
private bool AsyncOperationIsActive => _activeAsyncOperation != 0;
1063-
10641062
private void EnsureNoActiveAsyncOperation()
10651063
{
1066-
if (AsyncOperationIsActive)
1064+
if (_activeAsyncOperation)
1065+
{
10671066
ThrowInvalidBeginCall();
1067+
}
10681068
}
10691069

10701070
private void AsyncOperationStarting()
10711071
{
1072-
if (Interlocked.Exchange(ref _activeAsyncOperation, 1) != 0)
1072+
if (Interlocked.Exchange(ref _activeAsyncOperation, true))
10731073
{
10741074
ThrowInvalidBeginCall();
10751075
}
10761076
}
10771077

1078-
private void AsyncOperationCompleting() =>
1079-
Volatile.Write(ref _activeAsyncOperation, 0);
1078+
private void AsyncOperationCompleting()
1079+
{
1080+
Debug.Assert(_activeAsyncOperation);
1081+
_activeAsyncOperation = false;
1082+
}
10801083

10811084
private static void ThrowInvalidBeginCall() =>
10821085
throw new InvalidOperationException(SR.InvalidBeginCall);

0 commit comments

Comments
 (0)