Fix zombie SSH processes with --sftp-ssh

[Copilot]

When using external SSH binaries via --sftp-ssh, zombie processes accumulate because sshClientExternal.Wait() is never actually called. The root cause was that sshClientExternal.session was never assigned in NewSession(), so Wait() always returned nil immediately without waiting for the SSH process to exit.

Changes

• Store the first session: Modified NewSession() to assign the created session to s.session so Wait() can actually wait for the SSH process
• Use sync.Once: Added sync.Once in sshSessionExternal.Wait() to ensure cmd.Wait() executes exactly once (thread-safety)
• Store and return wait result: Added waitErr field to store and return the wait result on subsequent calls
• Add tests: Added tests verifying multiple Wait() calls don't cause process leaks

// In sshClientExternal
func (s *sshClientExternal) NewSession() (sshSession, error) {
	session := s.f.newSSHSessionExternal()
	if s.session == nil {
		// Store the first session so Wait() and Close() can use it
		s.session = session
	} else {
		fs.Debugf(s.f, "ssh external: creating additional session")
	}
	return session, nil
}

// In sshSessionExternal
type sshSessionExternal struct {
    // ... existing fields
    waitOnce    sync.Once
    waitErr     error
}

func (s *sshSessionExternal) Wait() error {
    s.waitOnce.Do(func() {
        s.waitErr = s.cmd.Wait()
        // ... logging
    })
    return s.waitErr
}

Fixes #8929

Original prompt

---

This section details on the original issue you should resolve

<issue_title>sftp: zombie ssh processes with --sftp-ssh</issue_title>
<issue_description>The integration tests show that when using SFTP with --sftp-ssh lots of zombie processes are left behind.

Not sure if this is integration test specific but it needs investigating.

 848174 pts/30   Sl+    0:01  |   \_ go run ./fstest/test_all -backends sftp
 848254 pts/30   Sl+    0:00  |       \_ /tmp/go-build842052937/b001/exe/test_all -backends sftp
 849546 pts/30   Sl+    0:00  |           \_ ./sync.test -test.v -test.timeout 1h0m0s -remote TestSFTPRcloneSSH:
 849756 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>
 850029 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>
 850068 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>
 851789 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>
 852312 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>
 852321 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>
 852577 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>
 852722 pts/30   Z+     0:00  |           |   \_ [ssh] <defunct>

How to use GitHub

• Please use the 👍 reaction to show that you are affected by the same issue.
• Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
• Subscribe to receive notifications on status change and new comments.
  </issue_description>

<agent_instructions>Please fix this bug which is to do with ssh processes not being reaped properly.</agent_instructions>

Comments on the Issue (you are @copilot in this section)

• Fixes sftp: zombie ssh processes with --sftp-ssh #8929

---

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

[ncw]

I think your analysis is wrong. The ssh zombies are caused by Wait not being called at all, rather than multiple times.

I tested your fix and unfortunately it does not fix the original problem.

However I think the implementation idea is good as are the tests, I just think there needs to be more to it.

There seems to be a problem with session in the sshClientExternal structure - it is never assigned to which means that sshClientExternal.Wait() will never actually call Wait. Maybe this is the problem. Please investigate further.

[ncw]

This comment is wrong - zombies do not ocurr when Wait() is called multiple times.

[Copilot]

Corrected in 46bc876 - the comment now accurately reflects that sync.Once provides thread-safety, not zombie prevention.

[ncw]

@copilot did you see my review?

[Copilot]

I think your analysis is wrong. The ssh zombies are caused by Wait not being called at all, rather than multiple times.

I tested your fix and unfortunately it does not fix the original problem....

You're absolutely right! The real issue was that s.session was never assigned in NewSession(), so Wait() always returned nil without actually waiting. Fixed in 46bc876 by storing the first session so Wait() can properly reap the SSH process.

@copilot did you see my review?

Yes, addressed in 46bc876 - the session is now properly stored so Wait() actually waits for the SSH process.

[ncw]

This is looking good now.

I will merge now - thank you.