The real issue was that sshClientExternal.session was never assigned,
so Wait() always returned nil without waiting for the SSH process to exit.
This caused zombie processes because the process was never reaped.
The fix:
- Store the first session created in NewSession() to s.session
- This allows Wait() to actually wait for the SSH process
- The sync.Once pattern is still useful for thread-safety
- Updated comments to reflect the correct behavior
Fixes the zombie process issue reported in rclone/rclone#8929
Co-authored-by: ncw <536803+ncw@users.noreply.github.com>
Remove unnecessary exited() check - we should always call cmd.Wait()
in the sync.Once block to properly reap the process and capture its
exit status. The sync.Once ensures it's only called once.
Co-authored-by: ncw <536803+ncw@users.noreply.github.com>
The issue was that cmd.Wait() was being called multiple times on the same
process - once in the background goroutine and once in Close(). This could
lead to zombie processes because only the first call to Wait() properly
reaps the process.
The fix uses sync.Once to ensure Wait() is only called once per SSH process,
storing and returning the result on subsequent calls.
Added tests to verify the fix works correctly.
Co-authored-by: ncw <536803+ncw@users.noreply.github.com>
This commit modernizes Go usage. This was done with:
go run golang.org/x/tools/gopls/internal/analysis/modernize/cmd/modernize@latest -fix -test ./...
Then files needed to be `go fmt`ed and a few comments needed to be
restored.
The modernizations include replacing
- if/else conditional assignment by a call to the built-in min or max functions added in go1.21
- sort.Slice(x, func(i, j int) bool) { return s[i] < s[j] } by a call to slices.Sort(s), added in go1.21
- interface{} by the 'any' type added in go1.18
- append([]T(nil), s...) by slices.Clone(s) or slices.Concat(s), added in go1.21
- loop around an m[k]=v map update by a call to one of the Collect, Copy, Clone, or Insert functions from the maps package, added in go1.21
- []byte(fmt.Sprintf...) by fmt.Appendf(nil, ...), added in go1.19
- append(s[:i], s[i+1]...) by slices.Delete(s, i, i+1), added in go1.21
- a 3-clause for i := 0; i < n; i++ {} loop by for i := range n {}, added in go1.22
This allows using an external ssh binary instead of the built in ssh
library for making SFTP connections.
This makes another integration test target TestSFTPRcloneSSH:
Fixes#7012