From 4911fb24722944243c1e80c61892344b6e9cdc5b Mon Sep 17 00:00:00 2001 From: Ilya Date: Wed, 26 Jun 2019 17:36:12 +0500 Subject: [PATCH 1/5] Use AddOrUpdate() to register runspace Fix style issue --- .../engine/runtime/Operations/ClassOps.cs | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs index c5de195132c..f275846e111 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs @@ -42,26 +42,18 @@ internal SessionStateKeeper() internal void RegisterRunspace() { - SessionStateInternal ssInMap = null; - Runspace rsToUse = Runspace.DefaultRunspace; - SessionStateInternal ssToUse = rsToUse.ExecutionContext.EngineSessionState; + Runspace runspaceToUse = Runspace.DefaultRunspace; + SessionStateInternal sessionStateToUse = runspaceToUse.ExecutionContext.EngineSessionState; // Different threads will operate on different key/value pairs (default-runspace/session-state pairs), // and a ConditionalWeakTable itself is thread safe, so there won't be race condition here. - if (!_stateMap.TryGetValue(rsToUse, out ssInMap)) - { - // If the key doesn't exist yet, add it - _stateMap.Add(rsToUse, ssToUse); - } - else if (!ssInMap.Equals(ssToUse)) - { - // If the key exists but the corresponding value is not what we should use, then remove the key/value pair and add the new pair. - // This could happen when a powershell class is defined in a module and the module gets reloaded. In such case, the same TypeDefinitionAst - // instance will get reused, but should be associated with the SessionState from the new module, instead of the one from the old module. - _stateMap.Remove(rsToUse); - _stateMap.Add(rsToUse, ssToUse); - } + + // If the key exists but the corresponding value is not what we should use, then update the key/value pair. + // This could happen when a powershell class is defined in a module and the module gets reloaded. + // In such case, the same TypeDefinitionAst instance will get reused, + // but should be associated with the SessionState from the new module, instead of the one from the old module. // If the key exists and the corresponding value is the one we should use, then do nothing. + _stateMap.AddOrUpdate(runspaceToUse, sessionStateToUse); } /// From db3eb76112a07d16e84110fba4b8f3d3a0a445f5 Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 27 Jun 2019 08:02:17 +0500 Subject: [PATCH 2/5] Revert "Use AddOrUpdate() to register runspace" This reverts commit 4911fb24722944243c1e80c61892344b6e9cdc5b. --- .../engine/runtime/Operations/ClassOps.cs | 24 ++++++++++++------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs index f275846e111..c5de195132c 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs @@ -42,18 +42,26 @@ internal SessionStateKeeper() internal void RegisterRunspace() { - Runspace runspaceToUse = Runspace.DefaultRunspace; - SessionStateInternal sessionStateToUse = runspaceToUse.ExecutionContext.EngineSessionState; + SessionStateInternal ssInMap = null; + Runspace rsToUse = Runspace.DefaultRunspace; + SessionStateInternal ssToUse = rsToUse.ExecutionContext.EngineSessionState; // Different threads will operate on different key/value pairs (default-runspace/session-state pairs), // and a ConditionalWeakTable itself is thread safe, so there won't be race condition here. - - // If the key exists but the corresponding value is not what we should use, then update the key/value pair. - // This could happen when a powershell class is defined in a module and the module gets reloaded. - // In such case, the same TypeDefinitionAst instance will get reused, - // but should be associated with the SessionState from the new module, instead of the one from the old module. + if (!_stateMap.TryGetValue(rsToUse, out ssInMap)) + { + // If the key doesn't exist yet, add it + _stateMap.Add(rsToUse, ssToUse); + } + else if (!ssInMap.Equals(ssToUse)) + { + // If the key exists but the corresponding value is not what we should use, then remove the key/value pair and add the new pair. + // This could happen when a powershell class is defined in a module and the module gets reloaded. In such case, the same TypeDefinitionAst + // instance will get reused, but should be associated with the SessionState from the new module, instead of the one from the old module. + _stateMap.Remove(rsToUse); + _stateMap.Add(rsToUse, ssToUse); + } // If the key exists and the corresponding value is the one we should use, then do nothing. - _stateMap.AddOrUpdate(runspaceToUse, sessionStateToUse); } /// From b74573e2d7473519bec03bec0c51d6b59f2a01bc Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 27 Jun 2019 08:11:22 +0500 Subject: [PATCH 3/5] Replace Remove() and Add() with AddOrUpdate() --- .../engine/runtime/Operations/ClassOps.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs index c5de195132c..245fe38f325 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs @@ -53,13 +53,12 @@ internal void RegisterRunspace() // If the key doesn't exist yet, add it _stateMap.Add(rsToUse, ssToUse); } - else if (!ssInMap.Equals(ssToUse)) + else if (!Object.ReferenceEquals(ssInMap, ssToUse)) { // If the key exists but the corresponding value is not what we should use, then remove the key/value pair and add the new pair. // This could happen when a powershell class is defined in a module and the module gets reloaded. In such case, the same TypeDefinitionAst // instance will get reused, but should be associated with the SessionState from the new module, instead of the one from the old module. - _stateMap.Remove(rsToUse); - _stateMap.Add(rsToUse, ssToUse); + _stateMap.AddOrUpdate(rsToUse, ssToUse); } // If the key exists and the corresponding value is the one we should use, then do nothing. } From d7c30554dfbcabaa50bcc05ee9b532fa26ec9164 Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 27 Jun 2019 08:13:28 +0500 Subject: [PATCH 4/5] Rename variables to fix CodeFactor issues --- .../engine/runtime/Operations/ClassOps.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs index 245fe38f325..1e2a2c9facf 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs @@ -42,23 +42,23 @@ internal SessionStateKeeper() internal void RegisterRunspace() { - SessionStateInternal ssInMap = null; - Runspace rsToUse = Runspace.DefaultRunspace; - SessionStateInternal ssToUse = rsToUse.ExecutionContext.EngineSessionState; + SessionStateInternal sessionStateInMap = null; + Runspace runspaceToUse = Runspace.DefaultRunspace; + SessionStateInternal sessionStateToUse = runspaceToUse.ExecutionContext.EngineSessionState; // Different threads will operate on different key/value pairs (default-runspace/session-state pairs), // and a ConditionalWeakTable itself is thread safe, so there won't be race condition here. - if (!_stateMap.TryGetValue(rsToUse, out ssInMap)) + if (!_stateMap.TryGetValue(runspaceToUse, out sessionStateInMap)) { // If the key doesn't exist yet, add it - _stateMap.Add(rsToUse, ssToUse); + _stateMap.Add(runspaceToUse, sessionStateToUse); } - else if (!Object.ReferenceEquals(ssInMap, ssToUse)) + else if (!Object.ReferenceEquals(sessionStateInMap, sessionStateToUse)) { // If the key exists but the corresponding value is not what we should use, then remove the key/value pair and add the new pair. // This could happen when a powershell class is defined in a module and the module gets reloaded. In such case, the same TypeDefinitionAst // instance will get reused, but should be associated with the SessionState from the new module, instead of the one from the old module. - _stateMap.AddOrUpdate(rsToUse, ssToUse); + _stateMap.AddOrUpdate(runspaceToUse, sessionStateToUse); } // If the key exists and the corresponding value is the one we should use, then do nothing. } From e603560f18e108f0c973e0b7f6c8093bd3dc11cb Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 27 Jun 2019 08:17:55 +0500 Subject: [PATCH 5/5] Correct compare --- .../engine/runtime/Operations/ClassOps.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs index 1e2a2c9facf..be30fe10310 100644 --- a/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs +++ b/src/System.Management.Automation/engine/runtime/Operations/ClassOps.cs @@ -53,7 +53,7 @@ internal void RegisterRunspace() // If the key doesn't exist yet, add it _stateMap.Add(runspaceToUse, sessionStateToUse); } - else if (!Object.ReferenceEquals(sessionStateInMap, sessionStateToUse)) + else if (sessionStateInMap != sessionStateToUse) { // If the key exists but the corresponding value is not what we should use, then remove the key/value pair and add the new pair. // This could happen when a powershell class is defined in a module and the module gets reloaded. In such case, the same TypeDefinitionAst