From 201e7e6f6dc20ce0e789f7cf59985954ed36187d Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 22 Jan 2019 20:51:52 +0100 Subject: [PATCH 01/14] flow Thread.CurrentPrincipal with ExecutionContext --- .../System/Threading/ThreadTestHelpers.cs | 5 ++ .../src/System/Threading/Thread.cs | 35 ++++++++++++- .../tests/ThreadTests.cs | 49 +++++++++++++++++++ 3 files changed, 87 insertions(+), 2 deletions(-) diff --git a/src/Common/tests/System/Threading/ThreadTestHelpers.cs b/src/Common/tests/System/Threading/ThreadTestHelpers.cs index 316fb1b9af9f..26e9fba5c34f 100644 --- a/src/Common/tests/System/Threading/ThreadTestHelpers.cs +++ b/src/Common/tests/System/Threading/ThreadTestHelpers.cs @@ -107,6 +107,11 @@ public static void RunTestInBackgroundThread(Action test) waitForThread(); } + public static void RunTestInBackgroundThread(Func test) + { + RunTestInBackgroundThread(() => test().Wait()); + } + public static void WaitForCondition(Func condition) { WaitForConditionWithCustomDelay(condition, () => Thread.Sleep(1)); diff --git a/src/System.Threading.Thread/src/System/Threading/Thread.cs b/src/System.Threading.Thread/src/System/Threading/Thread.cs index 0be10726c2f6..e94647ef526c 100644 --- a/src/System.Threading.Thread/src/System/Threading/Thread.cs +++ b/src/System.Threading.Thread/src/System/Threading/Thread.cs @@ -21,6 +21,7 @@ public sealed partial class Thread : CriticalFinalizerObject private CultureInfo _startCulture; private CultureInfo _startUICulture; private IPrincipal _principal; + private static AsyncLocal s_asyncLocalPrincipal; private Thread(RuntimeThread runtimeThread) { @@ -190,14 +191,44 @@ public static IPrincipal CurrentPrincipal { get { - return CurrentThread._principal ?? (CurrentThread._principal = AppDomain.CurrentDomain.GetThreadPrincipal()); + if (CurrentThread._principal is null) + { + IPrincipal principal = AppDomain.CurrentDomain.GetThreadPrincipal(); + if (principal is null) + { + return null; + } + else + { + SetAsyncLocalPrincipal(principal); + } + } + return CurrentThread._principal; } set { - CurrentThread._principal = value; + SetAsyncLocalPrincipal(value); } } + private static void SetAsyncLocalPrincipal(IPrincipal principal) + { + if (s_asyncLocalPrincipal is null) + { + if (principal is null) + { + return; + } + Interlocked.CompareExchange(ref s_asyncLocalPrincipal, new AsyncLocal(CurrentPrincipalContextChanged), null); + } + s_asyncLocalPrincipal.Value = principal; + } + + private static void CurrentPrincipalContextChanged(AsyncLocalValueChangedArgs valueChangedHandler) + { + CurrentThread._principal = valueChangedHandler.CurrentValue; + } + public ExecutionContext ExecutionContext => ExecutionContext.Capture(); public bool IsAlive => _runtimeThread.IsAlive; public bool IsBackground { get { return _runtimeThread.IsBackground; } set { _runtimeThread.IsBackground = value; } } diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 4559b4b2a50a..c1f99926dcdf 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -7,7 +7,9 @@ using System.Globalization; using System.Reflection; using System.Runtime.InteropServices; +using System.Security.Claims; using System.Security.Principal; +using System.Threading.Tasks; using System.Threading.Tests; using Xunit; @@ -427,6 +429,53 @@ public static void CurrentPrincipalTest() }); } + [Fact] + public static void CurrentPrincipalContextFlowTest() + { + ThreadTestHelpers.RunTestInBackgroundThread(async () => + { + Thread.CurrentPrincipal = new ClaimsPrincipal(); + + await Task.Run(async() => { + + Assert.IsType(Thread.CurrentPrincipal); + + await Task.Run(async() => + { + Assert.IsType(Thread.CurrentPrincipal); + + Thread.CurrentPrincipal = new GenericPrincipal(new GenericIdentity("name"), new string[0]); + + await Task.Run(() => + { + Assert.IsType(Thread.CurrentPrincipal); + }); + + Assert.IsType(Thread.CurrentPrincipal); + }); + + Assert.IsType(Thread.CurrentPrincipal); + }); + + Assert.IsType(Thread.CurrentPrincipal); + }); + } + + [Fact] + public static void CurrentPrincipalContextFlowTest_NotFlow() + { + ThreadTestHelpers.RunTestInBackgroundThread(async () => + { + Thread.CurrentPrincipal = new ClaimsPrincipal(); + + ExecutionContext.SuppressFlow(); + + await Task.Run(() => Assert.Null(Thread.CurrentPrincipal)); + + Assert.Null(Thread.CurrentPrincipal); + }); + } + [Fact] public static void CurrentThreadTest() { From 24a7ad39350e8106d5e9b04b8dbafd908330aedf Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 22 Jan 2019 21:29:33 +0100 Subject: [PATCH 02/14] fix netfx test --- src/System.Threading.Thread/tests/ThreadTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index c1f99926dcdf..653fd4b359bc 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -472,7 +472,8 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() await Task.Run(() => Assert.Null(Thread.CurrentPrincipal)); - Assert.Null(Thread.CurrentPrincipal); + // defaults are null for netcorepp and GenericPrincipal for netfx + Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is GenericPrincipal); }); } From 72f65d0bacbbb10b9f8d0996433d19f2629a04d8 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 23 Jan 2019 15:28:46 +0100 Subject: [PATCH 03/14] address PR feedback --- .../src/System/Threading/Thread.cs | 39 +++++++------------ .../tests/ThreadTests.cs | 8 +++- 2 files changed, 20 insertions(+), 27 deletions(-) diff --git a/src/System.Threading.Thread/src/System/Threading/Thread.cs b/src/System.Threading.Thread/src/System/Threading/Thread.cs index e94647ef526c..d54f34583aeb 100644 --- a/src/System.Threading.Thread/src/System/Threading/Thread.cs +++ b/src/System.Threading.Thread/src/System/Threading/Thread.cs @@ -16,11 +16,13 @@ public sealed partial class Thread : CriticalFinalizerObject [ThreadStatic] private static Thread t_currentThread; + [ThreadStatic] + private static IPrincipal s_principal; + private readonly RuntimeThread _runtimeThread; private Delegate _start; private CultureInfo _startCulture; private CultureInfo _startUICulture; - private IPrincipal _principal; private static AsyncLocal s_asyncLocalPrincipal; private Thread(RuntimeThread runtimeThread) @@ -191,42 +193,29 @@ public static IPrincipal CurrentPrincipal { get { - if (CurrentThread._principal is null) + if (s_principal is null) { - IPrincipal principal = AppDomain.CurrentDomain.GetThreadPrincipal(); - if (principal is null) - { - return null; - } - else - { - SetAsyncLocalPrincipal(principal); - } + CurrentPrincipal = AppDomain.CurrentDomain.GetThreadPrincipal(); } - return CurrentThread._principal; + return s_principal; } set { - SetAsyncLocalPrincipal(value); - } - } - - private static void SetAsyncLocalPrincipal(IPrincipal principal) - { - if (s_asyncLocalPrincipal is null) - { - if (principal is null) + if (s_asyncLocalPrincipal is null) { - return; + if (value is null) + { + return; + } + Interlocked.CompareExchange(ref s_asyncLocalPrincipal, new AsyncLocal(CurrentPrincipalContextChanged), null); } - Interlocked.CompareExchange(ref s_asyncLocalPrincipal, new AsyncLocal(CurrentPrincipalContextChanged), null); + s_asyncLocalPrincipal.Value = value; } - s_asyncLocalPrincipal.Value = principal; } private static void CurrentPrincipalContextChanged(AsyncLocalValueChangedArgs valueChangedHandler) { - CurrentThread._principal = valueChangedHandler.CurrentValue; + s_principal = valueChangedHandler.CurrentValue; } public ExecutionContext ExecutionContext => ExecutionContext.Capture(); diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 653fd4b359bc..27f2b5f8b177 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -468,11 +468,15 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() { Thread.CurrentPrincipal = new ClaimsPrincipal(); + // TaskCreationOptions.LongRunning to have fresh new thread with clean execution context + await Task.Factory.StartNew(() => Assert.IsType(Thread.CurrentPrincipal), CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); + ExecutionContext.SuppressFlow(); - await Task.Run(() => Assert.Null(Thread.CurrentPrincipal)); + // TaskCreationOptions.LongRunning to have fresh new thread with clean execution context + await Task.Factory.StartNew(() => Assert.Null(Thread.CurrentPrincipal), CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); - // defaults are null for netcorepp and GenericPrincipal for netfx + // defaults are null for netcorapp and GenericPrincipal for netfx Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is GenericPrincipal); }); } From 8bf0ddda57f236f330e68001f247b2b2711e2ab7 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Wed, 23 Jan 2019 17:37:06 +0100 Subject: [PATCH 04/14] address PR feedback --- .../src/System/Threading/Thread.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/System.Threading.Thread/src/System/Threading/Thread.cs b/src/System.Threading.Thread/src/System/Threading/Thread.cs index d54f34583aeb..51e38edd59b3 100644 --- a/src/System.Threading.Thread/src/System/Threading/Thread.cs +++ b/src/System.Threading.Thread/src/System/Threading/Thread.cs @@ -17,13 +17,13 @@ public sealed partial class Thread : CriticalFinalizerObject private static Thread t_currentThread; [ThreadStatic] - private static IPrincipal s_principal; + private static IPrincipal t_principal; + private static AsyncLocal s_asyncLocalPrincipal; private readonly RuntimeThread _runtimeThread; private Delegate _start; private CultureInfo _startCulture; private CultureInfo _startUICulture; - private static AsyncLocal s_asyncLocalPrincipal; private Thread(RuntimeThread runtimeThread) { @@ -193,11 +193,11 @@ public static IPrincipal CurrentPrincipal { get { - if (s_principal is null) + if (t_principal is null) { CurrentPrincipal = AppDomain.CurrentDomain.GetThreadPrincipal(); } - return s_principal; + return t_principal; } set { @@ -215,7 +215,7 @@ public static IPrincipal CurrentPrincipal private static void CurrentPrincipalContextChanged(AsyncLocalValueChangedArgs valueChangedHandler) { - s_principal = valueChangedHandler.CurrentValue; + t_principal = valueChangedHandler.CurrentValue; } public ExecutionContext ExecutionContext => ExecutionContext.Capture(); From dd927003ca642856bc3370df5a33396fc1b3be02 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Thu, 24 Jan 2019 17:58:18 +0100 Subject: [PATCH 05/14] try to make test more reliable using StartNew(), removed from NetFramework --- src/System.Threading.Thread/tests/ThreadTests.cs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 27f2b5f8b177..8cb82b9b0844 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -462,6 +462,7 @@ await Task.Run(() => } [Fact] + [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "ExecutionContext.SuppressFlow() doesn't work for Task.Factory.StartNew")] public static void CurrentPrincipalContextFlowTest_NotFlow() { ThreadTestHelpers.RunTestInBackgroundThread(async () => @@ -474,10 +475,7 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() ExecutionContext.SuppressFlow(); // TaskCreationOptions.LongRunning to have fresh new thread with clean execution context - await Task.Factory.StartNew(() => Assert.Null(Thread.CurrentPrincipal), CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); - - // defaults are null for netcorapp and GenericPrincipal for netfx - Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is GenericPrincipal); + await Task.Factory.StartNew(() => Assert.True(Thread.CurrentPrincipal is null), CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); }); } From 6b9c4b9e03d874f1fa40c36e414798c05a4602e2 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 25 Jan 2019 16:14:57 +0100 Subject: [PATCH 06/14] address PR feedback --- .../tests/ThreadTests.cs | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 8cb82b9b0844..17eb66b1f909 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -462,21 +462,22 @@ await Task.Run(() => } [Fact] - [SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework, "ExecutionContext.SuppressFlow() doesn't work for Task.Factory.StartNew")] public static void CurrentPrincipalContextFlowTest_NotFlow() { - ThreadTestHelpers.RunTestInBackgroundThread(async () => + // We run test on remote process to avoid to return dirty context to thread pool + DummyClass.RemoteInvoke(() => { - Thread.CurrentPrincipal = new ClaimsPrincipal(); + ThreadTestHelpers.RunTestInBackgroundThread(async () => + { + Thread.CurrentPrincipal = new ClaimsPrincipal(); - // TaskCreationOptions.LongRunning to have fresh new thread with clean execution context - await Task.Factory.StartNew(() => Assert.IsType(Thread.CurrentPrincipal), CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); + ExecutionContext.SuppressFlow(); - ExecutionContext.SuppressFlow(); + // Default principal policy for netcoreapp is null and for netfx is GenericPrincipal + await Task.Run(() => Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is GenericPrincipal)); + }); - // TaskCreationOptions.LongRunning to have fresh new thread with clean execution context - await Task.Factory.StartNew(() => Assert.True(Thread.CurrentPrincipal is null), CancellationToken.None, TaskCreationOptions.LongRunning, TaskScheduler.Default); - }); + }).Dispose(); } [Fact] From 754b169694d2aaea779803b69cfc752c3a7c58bc Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 25 Jan 2019 16:21:28 +0100 Subject: [PATCH 07/14] nit: extraline --- src/System.Threading.Thread/tests/ThreadTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 17eb66b1f909..a12c87ec14e9 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -476,7 +476,6 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() // Default principal policy for netcoreapp is null and for netfx is GenericPrincipal await Task.Run(() => Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is GenericPrincipal)); }); - }).Dispose(); } From cbc6ba9cf4cc02dc38b0a36abb872c3a10b36ff9 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 25 Jan 2019 16:28:34 +0100 Subject: [PATCH 08/14] add null set test --- .../tests/ThreadTests.cs | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index a12c87ec14e9..91ca67081103 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -479,6 +479,32 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() }).Dispose(); } + [Fact] + public static void CurrentPrincipalContextFlowTest_CurrentPrincipal_SetNull() + { + // We run test on remote process because we need to set same principal policy + // on netfx defaul principal policy is PrincipalPolicy.UnauthenticatedPrincipal + DummyClass.RemoteInvoke(() => + { + AppDomain.CurrentDomain.SetPrincipalPolicy(PrincipalPolicy.NoPrincipal); + + Assert.Null(Thread.CurrentPrincipal); + + Thread.CurrentPrincipal = null; + Assert.Null(Thread.CurrentPrincipal); + + Thread.CurrentPrincipal = new ClaimsPrincipal(); + Assert.IsType(Thread.CurrentPrincipal); + + Thread.CurrentPrincipal = null; + Assert.Null(Thread.CurrentPrincipal); + + Thread.CurrentPrincipal = new ClaimsPrincipal(); + Assert.IsType(Thread.CurrentPrincipal); + + }).Dispose(); + } + [Fact] public static void CurrentThreadTest() { From 9cc413cdd132ac3bc2316f2182f7d15881acde9e Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 25 Jan 2019 16:30:41 +0100 Subject: [PATCH 09/14] nit: extraline, again... --- src/System.Threading.Thread/tests/ThreadTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 91ca67081103..4df2e02a8f02 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -501,7 +501,6 @@ public static void CurrentPrincipalContextFlowTest_CurrentPrincipal_SetNull() Thread.CurrentPrincipal = new ClaimsPrincipal(); Assert.IsType(Thread.CurrentPrincipal); - }).Dispose(); } From 60848b7353ac4812ea986d61a72bed5b07963d15 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 25 Jan 2019 16:31:45 +0100 Subject: [PATCH 10/14] rename test --- src/System.Threading.Thread/tests/ThreadTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 4df2e02a8f02..ce238eafea47 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -480,7 +480,7 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() } [Fact] - public static void CurrentPrincipalContextFlowTest_CurrentPrincipal_SetNull() + public static void CurrentPrincipal_SetNull() { // We run test on remote process because we need to set same principal policy // on netfx defaul principal policy is PrincipalPolicy.UnauthenticatedPrincipal From 89a60d03a9ab9a48ca349ffe97140278244f95cc Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Fri, 25 Jan 2019 17:16:17 +0100 Subject: [PATCH 11/14] apply Stephen fix --- .../tests/ThreadTests.cs | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index ce238eafea47..7bd6b96be31e 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -464,19 +464,27 @@ await Task.Run(() => [Fact] public static void CurrentPrincipalContextFlowTest_NotFlow() { - // We run test on remote process to avoid to return dirty context to thread pool - DummyClass.RemoteInvoke(() => + ThreadTestHelpers.RunTestInBackgroundThread(async () => { - ThreadTestHelpers.RunTestInBackgroundThread(async () => + Thread.CurrentPrincipal = new ClaimsPrincipal(); + + Task task; + using(ExecutionContext.SuppressFlow()) { - Thread.CurrentPrincipal = new ClaimsPrincipal(); + Assert.True(ExecutionContext.IsFlowSuppressed()); - ExecutionContext.SuppressFlow(); + task = Task.Run(() => + { + // Default PrincipalPolicy for netcoreapp is null and for netfx is ClaimsPrincipal + Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is ClaimsPrincipal); + Assert.False(ExecutionContext.IsFlowSuppressed()); + }); + } - // Default principal policy for netcoreapp is null and for netfx is GenericPrincipal - await Task.Run(() => Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is GenericPrincipal)); - }); - }).Dispose(); + Assert.False(ExecutionContext.IsFlowSuppressed()); + + await task; + }); } [Fact] From f5b3bf32f52be6c4be131fa17118d103622cb7fd Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Mon, 28 Jan 2019 21:20:18 +0100 Subject: [PATCH 12/14] nit: typos --- src/System.Threading.Thread/tests/ThreadTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 7bd6b96be31e..62627a94bc74 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -491,7 +491,7 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() public static void CurrentPrincipal_SetNull() { // We run test on remote process because we need to set same principal policy - // on netfx defaul principal policy is PrincipalPolicy.UnauthenticatedPrincipal + // On netfx default principal policy is PrincipalPolicy.UnauthenticatedPrincipal DummyClass.RemoteInvoke(() => { AppDomain.CurrentDomain.SetPrincipalPolicy(PrincipalPolicy.NoPrincipal); From 7c2b448e545f5cb6ed369a534364e5e09b572343 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 29 Jan 2019 22:21:27 +0100 Subject: [PATCH 13/14] address PR feedback --- .../src/System/Threading/Thread.cs | 16 ++++------------ src/System.Threading.Thread/tests/ThreadTests.cs | 12 ++++++++++-- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/System.Threading.Thread/src/System/Threading/Thread.cs b/src/System.Threading.Thread/src/System/Threading/Thread.cs index 51e38edd59b3..e9e654a60233 100644 --- a/src/System.Threading.Thread/src/System/Threading/Thread.cs +++ b/src/System.Threading.Thread/src/System/Threading/Thread.cs @@ -15,9 +15,6 @@ public sealed partial class Thread : CriticalFinalizerObject { [ThreadStatic] private static Thread t_currentThread; - - [ThreadStatic] - private static IPrincipal t_principal; private static AsyncLocal s_asyncLocalPrincipal; private readonly RuntimeThread _runtimeThread; @@ -193,11 +190,11 @@ public static IPrincipal CurrentPrincipal { get { - if (t_principal is null) + if (s_asyncLocalPrincipal is null) { - CurrentPrincipal = AppDomain.CurrentDomain.GetThreadPrincipal(); + CurrentPrincipal = AppDomain.CurrentDomain.GetThreadPrincipal(); } - return t_principal; + return s_asyncLocalPrincipal?.Value; } set { @@ -207,17 +204,12 @@ public static IPrincipal CurrentPrincipal { return; } - Interlocked.CompareExchange(ref s_asyncLocalPrincipal, new AsyncLocal(CurrentPrincipalContextChanged), null); + Interlocked.CompareExchange(ref s_asyncLocalPrincipal, new AsyncLocal(), null); } s_asyncLocalPrincipal.Value = value; } } - private static void CurrentPrincipalContextChanged(AsyncLocalValueChangedArgs valueChangedHandler) - { - t_principal = valueChangedHandler.CurrentValue; - } - public ExecutionContext ExecutionContext => ExecutionContext.Capture(); public bool IsAlive => _runtimeThread.IsAlive; public bool IsBackground { get { return _runtimeThread.IsBackground; } set { _runtimeThread.IsBackground = value; } } diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 62627a94bc74..5437b003e82d 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -475,8 +475,16 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() task = Task.Run(() => { - // Default PrincipalPolicy for netcoreapp is null and for netfx is ClaimsPrincipal - Assert.True(Thread.CurrentPrincipal is null || Thread.CurrentPrincipal is ClaimsPrincipal); + // Default PrincipalPolicy for netcoreapp is null and for netfx is GenericPrincipal + if (PlatformDetection.IsNetCore) + { + Assert.Null(Thread.CurrentPrincipal); + } + else + { + Assert.IsType(Thread.CurrentPrincipal); + } + Assert.False(ExecutionContext.IsFlowSuppressed()); }); } From 9edc53c5bf411af5f25a65a6f6ac46cea11f7153 Mon Sep 17 00:00:00 2001 From: Marco Rossignoli Date: Tue, 29 Jan 2019 22:24:01 +0100 Subject: [PATCH 14/14] nit: update comment --- src/System.Threading.Thread/tests/ThreadTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Threading.Thread/tests/ThreadTests.cs b/src/System.Threading.Thread/tests/ThreadTests.cs index 5437b003e82d..facea368d6d2 100644 --- a/src/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/System.Threading.Thread/tests/ThreadTests.cs @@ -475,7 +475,7 @@ public static void CurrentPrincipalContextFlowTest_NotFlow() task = Task.Run(() => { - // Default PrincipalPolicy for netcoreapp is null and for netfx is GenericPrincipal + // Default PrincipalPolicy for netcoreapp is null if (PlatformDetection.IsNetCore) { Assert.Null(Thread.CurrentPrincipal);