Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 30 additions & 4 deletions src/System.Private.CoreLib/shared/System/Threading/ThreadLocal.cs
Original file line number Diff line number Diff line change
Expand Up @@ -454,16 +454,16 @@ public IList<T> Values
/// <summary>Gets all of the threads' values in a list.</summary>
private List<T>? GetValuesAsList()
{
List<T> valueList = new List<T>();
LinkedSlot? linkedSlot = _linkedSlot;
int id = ~_idComplement;
if (id == -1)
if (id == -1 || linkedSlot == null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fixing a bug when _linkedSlot? Do we need a test for it?

Copy link
Contributor Author

@kouvel kouvel Apr 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a possibility that after Dispose is called and another thread calls this method, because there are no restrictions on the loads here, the other thread could read an old value for _idComplement (where id != 0) and a new value for _linkedSlot (== null), then it would throw NullReferenceException instead of ObjectDisposedException. I didn't see anything that would prevent that, and thought it would be difficult to repro but it seems to be fairly easy, will add a test to the CoreFX PR.

{
return null;
}

// Walk over the linked list of slots and gather the values associated with this ThreadLocal instance.
Debug.Assert(_linkedSlot != null, "Should only be null if the instance was disposed.");
for (LinkedSlot? linkedSlot = _linkedSlot._next; linkedSlot != null; linkedSlot = linkedSlot._next)
var valueList = new List<T>();
for (linkedSlot = linkedSlot._next; linkedSlot != null; linkedSlot = linkedSlot._next)
{
// We can safely read linkedSlot.Value. Even if this ThreadLocal has been disposed in the meantime, the LinkedSlot
// objects will never be assigned to another ThreadLocal instance.
Expand All @@ -473,6 +473,32 @@ public IList<T> Values
return valueList;
}

internal IEnumerable<T> ValuesAsEnumerable
{
get
{
if (!_trackAllValues)
{
throw new InvalidOperationException(SR.ThreadLocal_ValuesNotAvailable);
}

LinkedSlot? linkedSlot = _linkedSlot;
int id = ~_idComplement;
if (id == -1 || linkedSlot == null)
{
throw new ObjectDisposedException(SR.ThreadLocal_Disposed);
}

// Walk over the linked list of slots and gather the values associated with this ThreadLocal instance.
for (linkedSlot = linkedSlot._next; linkedSlot != null; linkedSlot = linkedSlot._next)
{
// We can safely read linkedSlot.Value. Even if this ThreadLocal has been disposed in the meantime, the LinkedSlot
// objects will never be assigned to another ThreadLocal instance.
yield return linkedSlot._value;
}
}
}

/// <summary>Gets the number of threads that have data in this instance.</summary>
private int ValuesCountForDebugDisplay
{
Expand Down
2 changes: 2 additions & 0 deletions src/System.Private.CoreLib/src/System.Private.CoreLib.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,8 @@
<Compile Include="System\Threading\SyncTable.cs" Condition="'$(UseSyncTable)' == 'true'" />
<Compile Include="System\Threading\Tasks\AsyncCausalityTracer.WinRT.cs" Condition="'$(EnableWinRT)' == 'true'" />
<Compile Include="System\Threading\Thread.CoreRT.cs" />
<Compile Include="System\Threading\ThreadBooleanCounter.cs" />
<Compile Include="System\Threading\ThreadInt64PersistentCounter.cs" />
<Compile Include="System\Threading\ThreadPool.CoreRT.cs" />
<Compile Include="System\Threading\ThreadPoolCallbackWrapper.cs" />
<Compile Include="System\Threading\WaitHandle.CoreRT.cs" />
Expand Down
12 changes: 7 additions & 5 deletions src/System.Private.CoreLib/src/System/Threading/ClrThreadPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ private struct CacheLineSeparated

private CacheLineSeparated _separated;
private ulong _currentSampleStartTime;
private int _completionCount = 0;
private readonly ThreadInt64PersistentCounter _completionCounter = new ThreadInt64PersistentCounter();
private int _threadAdjustmentIntervalMs;

private LowLevelLock _hillClimbingThreadAdjustmentLock = new LowLevelLock();
Expand Down Expand Up @@ -184,18 +184,20 @@ public bool SetMaxThreads(int maxThreads)
public int GetAvailableThreads()
{
ThreadCounts counts = ThreadCounts.VolatileReadCounts(ref _separated.counts);
int count = _maxThreads - counts.numExistingThreads;
int count = _maxThreads - counts.numProcessingWork;
if (count < 0)
{
return 0;
}
return count;
}

public int ThreadCount => ThreadCounts.VolatileReadCounts(ref _separated.counts).numExistingThreads;
public long CompletedWorkItemCount => _completionCounter.Count;

internal bool NotifyWorkItemComplete()
{
// TODO: Check perf. Might need to make this thread-local.
Interlocked.Increment(ref _completionCount);
_completionCounter.Increment();
Volatile.Write(ref _separated.lastDequeueTime, Environment.TickCount);

if (ShouldAdjustMaxWorkersActive() && _hillClimbingThreadAdjustmentLock.TryAcquire())
Expand All @@ -221,7 +223,7 @@ private void AdjustMaxWorkersActive()
{
_hillClimbingThreadAdjustmentLock.VerifyIsLocked();
int currentTicks = Environment.TickCount;
int totalNumCompletions = Volatile.Read(ref _completionCount);
int totalNumCompletions = (int)_completionCounter.Count;
int numCompletions = totalNumCompletions - _separated.priorCompletionCount;
ulong startTime = _currentSampleStartTime;
ulong endTime = HighPerformanceCounter.TickCount;
Expand Down
12 changes: 9 additions & 3 deletions src/System.Private.CoreLib/src/System/Threading/Lock.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ public bool TryAcquire(TimeSpan timeout)
return TryAcquire(WaitHandle.ToTimeoutMilliseconds(timeout));
}

public bool TryAcquire(int millisecondsTimeout)
public bool TryAcquire(int millisecondsTimeout, bool trackContentions = false)
{
if (millisecondsTimeout < -1)
throw new ArgumentOutOfRangeException(nameof(millisecondsTimeout), SR.ArgumentOutOfRange_NeedNonNegOrNegative1);
Expand All @@ -121,10 +121,10 @@ public bool TryAcquire(int millisecondsTimeout)
//
// Fall back to the slow path for contention
//
return TryAcquireContended(currentThreadId, millisecondsTimeout);
return TryAcquireContended(currentThreadId, millisecondsTimeout, trackContentions);
}

private bool TryAcquireContended(IntPtr currentThreadId, int millisecondsTimeout)
private bool TryAcquireContended(IntPtr currentThreadId, int millisecondsTimeout, bool trackContentions = false)
{
//
// If we already own the lock, just increment the recursion count.
Expand Down Expand Up @@ -185,6 +185,12 @@ private bool TryAcquireContended(IntPtr currentThreadId, int millisecondsTimeout
//
// Now we wait.
//

if (trackContentions)
{
Monitor.IncrementLockContentionCount();
}

TimeoutTracker timeoutTracker = TimeoutTracker.Start(millisecondsTimeout);
AutoResetEvent ev = Event;

Expand Down
16 changes: 15 additions & 1 deletion src/System.Private.CoreLib/src/System/Threading/Monitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ internal static bool TryAcquireContended(Lock lck, object obj, int millisecondsT

using (new DebugBlockingScope(obj, DebugBlockingItemType.MonitorCriticalSection, millisecondsTimeout, out blockingItem))
{
return lck.TryAcquire(millisecondsTimeout);
return lck.TryAcquire(millisecondsTimeout, trackContentions: true);
}
}

Expand Down Expand Up @@ -246,5 +246,19 @@ public void Dispose()
}

#endregion

#region Metrics

private static readonly ThreadInt64PersistentCounter s_lockContentionCounter = new ThreadInt64PersistentCounter();

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static void IncrementLockContentionCount() => s_lockContentionCounter.Increment();

/// <summary>
/// Gets the number of times there was contention upon trying to take a <see cref="Monitor"/>'s lock so far.
/// </summary>
public static long LockContentionCount => s_lockContentionCounter.Count;

#endregion
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ internal static void InitializeCom()
}

public void DisableComObjectEagerCleanup() { }
private static void InitializeExistingThreadPoolThread() { }

public void Interrupt() => WaitSubsystem.Interrupt(this);
internal static void UninterruptibleSleep0() => WaitSubsystem.UninterruptibleSleep0();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

using Microsoft.Win32.SafeHandles;
using System.Diagnostics;
using System.Runtime;
using System.Runtime.InteropServices;

namespace System.Threading
Expand Down Expand Up @@ -331,6 +332,13 @@ private static void UninitializeCom()

// TODO: https://github.com/dotnet/corefx/issues/20766
public void DisableComObjectEagerCleanup() { }

private static void InitializeExistingThreadPoolThread()
{
InitializeCom();
ThreadPool.InitializeForThreadPoolThread();
}

public void Interrupt() { throw new PlatformNotSupportedException(); }

internal static void UninterruptibleSleep0()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ private static Thread InitializeExistingThread(bool threadPoolThread)

if (threadPoolThread)
{
InitializeCom();
InitializeExistingThreadPoolThread();
}

return currentThread;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System.Diagnostics;
using System.Runtime.CompilerServices;

namespace System.Threading
{
internal sealed class ThreadBooleanCounter
{
private readonly ThreadLocal<bool> _threadLocalFlag = new ThreadLocal<bool>(trackAllValues: true);

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Set()
{
Debug.Assert(!_threadLocalFlag.Value);

try
{
_threadLocalFlag.Value = true;
}
catch (OutOfMemoryException)
{
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void Clear()
{
Debug.Assert(!_threadLocalFlag.IsValueCreated || _threadLocalFlag.Value);
_threadLocalFlag.Value = false;
}

public int Count
{
get
{
int count = 0;
try
{
foreach (bool isSet in _threadLocalFlag.ValuesAsEnumerable)
{
if (isSet)
{
++count;
Debug.Assert(count > 0);
}
}
return count;
}
catch (OutOfMemoryException)
{
// Some allocation occurs above and it may be a bit awkward to get an OOM from this property getter
return count;
}
}
}
}
}
Loading