Wait for SSH exit when closing remote connection

[dinhngtu]

PR Summary

Fixes #13356 by adding custom functions for killing and waiting for SSH processes. Requires PowerShell/PowerShell-Native#63.

PR Context

On Unix, after a SSH session finishes, PowerShell uses System.Diagnostics.Process.Kill() to stop the process, but doesn't reap the process with waitpid(). This causes SSH to become a defunct (zombie) process.

PR Checklist

• PR has a meaningful title
  • Use the present tense and imperative mood when describing your changes
• Summarized changes
• Make sure all .h, .cpp, .cs, .ps1 and .psm1 files have the correct copyright header
• This PR is ready to merge and is not Work in Progress.
  • If the PR is work in progress, please add the prefix WIP: or [ WIP ] to the beginning of the title (the WIP bot will keep its status check at Pending while the prefix is present) and remove the prefix when the PR is ready.
• Breaking changes
  • None
  • OR
  • Experimental feature(s) needed
    • Experimental feature name(s):
• User-facing changes
  • Not Applicable
  • OR
  • Documentation needed
    • Issue filed:
• Testing - New and feature
  • N/A or can only be tested interactively
  • OR
  • Make sure you've added a new test if existing tests do not effectively test the code changed
• Tooling
  • I have considered the user experience from a tooling perspective and don't believe tooling will be impacted.
  • OR
  • I have considered the user experience from a tooling perspective and enumerated concerns in the summary. This may include:
    • Impact on PowerShell Editor Services which is used in the PowerShell extension for VSCode (which runs in a different PS Host).
    • Impact on Completions (both in the console and in editors) - one of PowerShell's most powerful features.
    • Impact on PSScriptAnalyzer (which provides linting & formatting in the editor extensions).
    • Impact on EditorSyntax (which provides syntax highlighting with in VSCode, GitHub, and many other editors).

[iSazonov]

Do we really need this in libpsl? Why not call from libc directly? Search by DllImport("libc" for examples.

Also we should add [return: MarshalAs(UnmanagedType.Bool)].

[dinhngtu]

Signals and other constants don't have a defined, standardized value, so I prefer to wrap it in libpsl and use the header constants there rather than coding it into S.M.A.

[iSazonov]

I don't see we use these constants - I see only 2 parameters and return value.

[dinhngtu]

These are referenced in libpsl wrappers: PowerShell/PowerShell-Native#63

If you called directly into libc from S.M.A then you'd need to call kill(pid, SIGKILL) and waitpid(pid, NULL, WNOHANG).

[iSazonov]

Aren't both POSIX?

[dinhngtu]

POSIX doesn't define the specific values of either constants (though SIGKILL should equal 9 most of the time).

[iSazonov]

Do they really different on supported platforms?

[dinhngtu]

I haven't looked into all platforms but following POSIX should be more portable than hardcoding the constants in S.M.A. While it might work with SIGKILL I doubt it'll be very reliable to hardcode WNOHANG. In addition, if we directly imported kill()/waitpid() in S.M.A it would then be possible to use them directly with nonportable constants, so I prefer to restrict the call interface in this fashion.

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

[adityapatwardhan]

@dinhngtu Just merged the PR in PowerShell Native.

[dinhngtu]

@dinhngtu Just merged the PR in PowerShell Native.

Should I update something in powershell to use the new libpsl? I had to manually replace libpsl-native.so to test the new native library.

[iSazonov]

@dinhngtu It is still unpublished https://www.nuget.org/packages/Microsoft.PowerShell.Native/

Microsoft.PowerShell.Native 7.1.0

Native binaries for PowerShell Core

[iSazonov]

@dinhngtu 7.2.0-preview.2 was published - please update the reference in csproj file.

[iSazonov]

@daxian-dbw @TravisEz13 Could you please clarify what is "1 workflow awaiting approval"? Is it CodeQL? Can I approve?

[ghost]

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

[iSazonov]

@dinhngtu Thanks for your contribution!

[ghost]

🎉v7.2.0-preview.7 has been released which incorporates this pull request.:tada:

Handy links:

• Release Notes