GeezerVison

Steve Lee (@SteveL-MSFT)

Regarding the server named pipe dispose, this is done in WindowsPowerShell via the CLR AppDomain unload event, which is not supported on CORECLR. However, you can use the AppDomain.CurrentDomain.ProcessExit event that we use for AMSI un-initializing. Unfortunately, these CLR events are not reliable, but they are better than nothing.

The start time added to the filename is intended to make the named pipe instance unique, to prevent a DOS or man-in-the-middle attack. We will want to keep something like this, even if it means using something shorter.

I love when a PR has more deletions than insertions yet more functionality

As for the domain name, do we really need it for the pipe name? Is multiple domain is going to be supported in .NET Core 3.0?

I have no problem keeping it for now until we get an accurate answer for the app domain support.

Dongbo Wang (@daxian-dbw) best I can tell, AppDomains are deprecated in .Net Core and not in the roadmap I can see for 3.0. So it may be fine to remove it for non-Windows. We'll have to keep it for Windows so that Windows PowerShell and PowerShell Core interop still works however.

Yes, I have proposed dropping the Domain name. If it gets supported again in dotNet Core then we can add it back.

To retain interoperability between Windows PowerShell and keep the code similar between Windows and non-Windows, I'll shorten the DefaultAppDomainName to 'none' but will keep it in the pipe name.

Steve Lee (@SteveL-MSFT) I forgot about maintaining compatibility with Windows PowerShell. We definitely need to ensure we can still list/parse/connect to those hosts.

Ugh, with the changes I've made since, I can't connect to/from Windows PowerShell and PowerShell Core. Debugging.

Looks like DefaultAppDomain is necessary on Windows as there is a check for that string and the starttime needs to stay as filetime integers on Windows to support interop between Windows PowerShell and PowerShell Core. I believe everything else is addressed. Paul Higinbotham (@PaulHigin) can you take another look?

Steve Lee (@SteveL-MSFT) Yeah, for Windows we need to keep the DomainName to be able to connect to WindowsPowerShell host, and that means keeping the default domain name for Windows platform pipe names.

Paul Higinbotham (@PaulHigin) yes, the recent commit has the same DefaultAppDomainName and starttime format on Windows and I verified manually it works from and to Windows PowerShell

Paul Higinbotham (@PaulHigin) good suggestion. Let me add a test for Windows PowerShell interop.

Steve Lee (@SteveL-MSFT), there are tests about Enter-PSHostProcess failing in AppVeyor, can you please take a look?

Dongbo Wang (@daxian-dbw) it appears to be a problem with redirected input with PSReadLine and only on AppVeyor. We're moving off AppVeyor (eventually), so I think we can ignore this otherwise we'll have to special case this by perhaps removing PSReadLine for this test

Given Steve Lee (@SteveL-MSFT)'s clarification on the AppVeyor test failure, I'm merging this PR.