From 58a799652ee8718dd3cc5566747f1192cf54d531 Mon Sep 17 00:00:00 2001 From: PetSerAl Date: Sun, 26 Mar 2017 13:49:05 +0300 Subject: [PATCH 1/4] Modification to TranscribeOnly behavior --- .../FormatAndOutput/out-console/OutConsole.cs | 23 ++++++++++++++---- .../engine/hostifaces/MshHostUserInterface.cs | 24 ++++++++++++++++++- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs b/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs index 484e9967439..65aa95bacfb 100644 --- a/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs +++ b/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs @@ -79,10 +79,9 @@ protected override void BeginProcessing() mrt.MergeUnclaimedPreviousErrorResults = true; } - _savedTranscribeOnly = Host.UI.TranscribeOnly; if (Transcript) { - Host.UI.TranscribeOnly = true; + transcribeOnlyCookie = Host.UI.SetTranscribeOnly(); } // This needs to be done directly through the command runtime, as Out-Default @@ -143,15 +142,29 @@ protected override void EndProcessing() } base.EndProcessing(); + } - if (Transcript) + /// + /// Revert transcription state on Dispose + /// + protected override void InternalDispose() + { + try { - Host.UI.TranscribeOnly = _savedTranscribeOnly; + base.InternalDispose(); + } + finally + { + if (transcribeOnlyCookie != null) + { + transcribeOnlyCookie.Dispose(); + transcribeOnlyCookie = null; + } } } private ArrayList _outVarResults = null; - private bool _savedTranscribeOnly = false; + private IDisposable transcribeOnlyCookie = null; } /// diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 8249eb33d01..a8c601a0f04 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -390,7 +390,29 @@ internal void IgnoreCommand(string commandText, InvocationInfo invocation) /// so that when content is sent through Out-Default it doesn't /// make it to the actual host. /// - internal bool TranscribeOnly { get; set; } + internal bool TranscribeOnly => transcribeOnlyCount != 0; + private int transcribeOnlyCount = 0; + internal IDisposable SetTranscribeOnly() => new TranscribeOnlyCookie(this); + private sealed class TranscribeOnlyCookie : IDisposable + { + private PSHostUserInterface ui; + private bool disposed = false; + public TranscribeOnlyCookie(PSHostUserInterface ui) + { + this.ui=ui; + ++ui.transcribeOnlyCount; + } + public void Dispose() + { + if (!disposed) + { + --ui.transcribeOnlyCount; + disposed = true; + GC.SuppressFinalize(this); + } + } + ~TranscribeOnlyCookie() => Dispose(); + } /// /// Flag to determine whether the host is transcribing. From fd76724e5b3b2bdce5837351f80f046632b2df1b Mon Sep 17 00:00:00 2001 From: PetSerAl Date: Sun, 26 Mar 2017 20:59:13 +0300 Subject: [PATCH 2/4] Use Interlocked for thread-safety with finalizer thread --- .../engine/hostifaces/MshHostUserInterface.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index a8c601a0f04..256edb7a246 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -11,6 +11,7 @@ using System.Globalization; using System.Management.Automation.Runspaces; using Microsoft.PowerShell.Commands; +using System.Threading; using System.Threading.Tasks; namespace System.Management.Automation.Host @@ -390,7 +391,7 @@ internal void IgnoreCommand(string commandText, InvocationInfo invocation) /// so that when content is sent through Out-Default it doesn't /// make it to the actual host. /// - internal bool TranscribeOnly => transcribeOnlyCount != 0; + internal bool TranscribeOnly => Interlocked.CompareExchange(ref transcribeOnlyCount, 0, 0) != 0; private int transcribeOnlyCount = 0; internal IDisposable SetTranscribeOnly() => new TranscribeOnlyCookie(this); private sealed class TranscribeOnlyCookie : IDisposable @@ -400,13 +401,13 @@ private sealed class TranscribeOnlyCookie : IDisposable public TranscribeOnlyCookie(PSHostUserInterface ui) { this.ui=ui; - ++ui.transcribeOnlyCount; + Interlocked.Increment(ref ui.transcribeOnlyCount); } public void Dispose() { if (!disposed) { - --ui.transcribeOnlyCount; + Interlocked.Decrement(ref ui.transcribeOnlyCount); disposed = true; GC.SuppressFinalize(this); } From 6799cac21bb0f03891ba11f094fc77ba22e391ec Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL)" Date: Mon, 27 Mar 2017 13:34:21 -0700 Subject: [PATCH 3/4] Add tests from #3392 --- .../Out-Default.Tests.ps1 | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) create mode 100644 test/powershell/Modules/Microsoft.PowerShell.Core/Out-Default.Tests.ps1 diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Out-Default.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Out-Default.Tests.ps1 new file mode 100644 index 00000000000..fa8ddc3dd0a --- /dev/null +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Out-Default.Tests.ps1 @@ -0,0 +1,40 @@ +Describe "Out-Default Tests" -tag CI { + BeforeAll { + # due to https://github.com/PowerShell/PowerShell/issues/3405, `Out-Default -Transcript` emits output to pipeline + # as running in Pester effectively wraps everything in parenthesis, workaround is to use another powershell + # to run the test script passed as a string + $powershell = "$PSHOME/powershell" + } + + It "'Out-Default -Transcript' shows up in transcript, but not host" { + $script = @" + `$null = Start-Transcript -Path "$testdrive\transcript.txt"; + 'hello' | Microsoft.PowerShell.Core\Out-Default -Transcript; + 'bye'; + `$null = Stop-Transcript +"@ + + & $powershell -c $script | Should BeExactly 'bye' + "TestDrive:\transcript.txt" | Should Contain 'hello' + } + + It "Out-Default reverts transcription state when used more than once in a pipeline" { + & $powershell -c "Out-Default -Transcript | Out-Default -Transcript; 'Hello'" | Should BeExactly "Hello" + } + + It "Out-Default reverts transcription state when exception occurs in pipeline" { + & $powershell -c "try { & { throw } | Out-Default -Transcript } catch {}; 'Hello'" | Should BeExactly "Hello" + } + + It "Out-Default reverts transcription state even if Dispose() isn't called" { + $script = @" + `$sp = {Out-Default -Transcript}.GetSteppablePipeline(); + `$sp.Begin(`$false); + `$sp = `$null; + [GC]::Collect(); + [GC]::WaitForPendingFinalizers(); + 'hello' +"@ + & $powershell -c $script | Should BeExactly 'hello' + } +} From ed6dc29bbfba9e383db40b290e8d7e6ab381f7c9 Mon Sep 17 00:00:00 2001 From: PetSerAl Date: Tue, 28 Mar 2017 03:13:50 +0300 Subject: [PATCH 4/4] Rename fields to follow convention --- .../FormatAndOutput/out-console/OutConsole.cs | 10 +++++----- .../engine/hostifaces/MshHostUserInterface.cs | 18 +++++++++--------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs b/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs index 65aa95bacfb..f685c43f369 100644 --- a/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs +++ b/src/System.Management.Automation/commands/utility/FormatAndOutput/out-console/OutConsole.cs @@ -81,7 +81,7 @@ protected override void BeginProcessing() if (Transcript) { - transcribeOnlyCookie = Host.UI.SetTranscribeOnly(); + _transcribeOnlyCookie = Host.UI.SetTranscribeOnly(); } // This needs to be done directly through the command runtime, as Out-Default @@ -155,16 +155,16 @@ protected override void InternalDispose() } finally { - if (transcribeOnlyCookie != null) + if (_transcribeOnlyCookie != null) { - transcribeOnlyCookie.Dispose(); - transcribeOnlyCookie = null; + _transcribeOnlyCookie.Dispose(); + _transcribeOnlyCookie = null; } } } private ArrayList _outVarResults = null; - private IDisposable transcribeOnlyCookie = null; + private IDisposable _transcribeOnlyCookie = null; } /// diff --git a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs index 256edb7a246..fc3bc4f97eb 100644 --- a/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs +++ b/src/System.Management.Automation/engine/hostifaces/MshHostUserInterface.cs @@ -391,24 +391,24 @@ internal void IgnoreCommand(string commandText, InvocationInfo invocation) /// so that when content is sent through Out-Default it doesn't /// make it to the actual host. /// - internal bool TranscribeOnly => Interlocked.CompareExchange(ref transcribeOnlyCount, 0, 0) != 0; - private int transcribeOnlyCount = 0; + internal bool TranscribeOnly => Interlocked.CompareExchange(ref _transcribeOnlyCount, 0, 0) != 0; + private int _transcribeOnlyCount = 0; internal IDisposable SetTranscribeOnly() => new TranscribeOnlyCookie(this); private sealed class TranscribeOnlyCookie : IDisposable { - private PSHostUserInterface ui; - private bool disposed = false; + private PSHostUserInterface _ui; + private bool _disposed = false; public TranscribeOnlyCookie(PSHostUserInterface ui) { - this.ui=ui; - Interlocked.Increment(ref ui.transcribeOnlyCount); + _ui=ui; + Interlocked.Increment(ref _ui._transcribeOnlyCount); } public void Dispose() { - if (!disposed) + if (!_disposed) { - Interlocked.Decrement(ref ui.transcribeOnlyCount); - disposed = true; + Interlocked.Decrement(ref _ui._transcribeOnlyCount); + _disposed = true; GC.SuppressFinalize(this); } }