Allow opt-out of the named-pipe listener using an environment variable #26086
Conversation
daxian-dbw
added
the
CL-Engine
|
|
||
| var boolStr = str.AsSpan(); | ||
|
|
||
| if (boolStr.Length == 1) |
There was a problem hiding this comment.
Could you possibly use SequenceEqual possibly with or without StringComparer.OrdinalIgnoreCase to avoid the individual length and char checks. I assume .NET has all the necessary logic to make that performant and you could make this a lot simpler by doing something like
ReadOnlySpan<char> true1 = ['1'];
ReadOnlySpan<char> trueYes = ['y', 'e', 's'];
ReadOnlySpan<char> trueTrue = ['t', 'r', 'u', 'e'];
// Repeat for false checks
var boolStr = str.AsSpan()
if (boolStr.SequenceEquals(true1, StringComparer.OrdinalIgnoreCase) ||
boolStr.SequenceEquals(trueYes, StringComparer.OrdinalIgnoreCase ||
...)
{
return true;
}
// Repeat for false checks and default
...Not sure if stackalloc'ing one big span with all the options and slicing it would be more efficient or not but that's probably in micro-optimising territory.
Edit: Ah I see this has been copied from elsewhere.
There was a problem hiding this comment.
Yes, this is copied from Telemetry.cs. I will keep it as is in this PR as this one will be included in the next preview and the following RC. But I will submit a separate PR to make this optimization change.
|
/azp run PowerShell-Windows-Packaging-CI, PowerShell-CI-linux-packaging |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
daxian-dbw
force-pushed
the
diag_optout
branch
from
d157bcd to
e68a510
Compare
|
@iSazonov I guess you clicked the "Update branch" button. There were unrelated test failures after merging the master branch, so I have to revert that (force pushed the original 2 commits again). Please approve again once all CIs pass, thank you! |
|
Oh, sorry! It seems the button is too near to scroll bar. I restarted CIs and the WindowStyle test fail again. :-( |
|
/azp run PowerShell-CI-linux-packaging, PowerShell-Windows-Packaging-CI |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
The failing tests are as following. They are not related to the changes and consistently fail for all CIs starting from 9/26. [UPDATE] I cannot bypass the policy to merge the PR with failing tests. The 3 tests are disabled now in master branch, and I will rebase the PR against the master branch. |
daxian-dbw
force-pushed
the
diag_optout
branch
from
8b1cf27 to
142cd55
Compare
|
/azp run PowerShell-CI-linux-packaging, PowerShell-Windows-Packaging-CI |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
Should the new env variable be documented? |
6 participants
PR Summary
Fix #11599
The named-pipe listener is created for all PowerShell instances today (including scenarios where apps host PowerShell). On Linux platforms, this causes the named pipe files left behind uncleaned when PowerShell terminates abruptly.
This PR includes changes to allow opt-out of the named-pipe listener using the environment variable
POWERSHELL_DIAGNOSTICS_OPTOUT.For reference, .NET has the environment variable DOTNET_EnableDiagnostics for disabling the debugger, the profiler, and
EventPipediagnostics (enabled by default).PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright header