mirror of
https://github.com/rclone/rclone.git
synced 2026-11-04 11:43:22 +00:00
Dirk Petersen
7181430fa1
When the sftp backend uses an external ssh program (--sftp-ssh), one goroutine waits for the ssh program to finish by calling cmd.Wait(). cmd.Wait() sets cmd.ProcessState when the program ends. At the same time, CanReuse() calls exited(), which reads cmd.ProcessState without any lock. This is a data race and the Go race detector reports it. It happens in normal use, for example when a connection is put back in the pool after a stat. The race only decides whether a pooled connection can be reused, so it is unlikely to cause problems in practice, but it makes programs that use the sftp backend fail when they are tested with -race. This fix sets an atomic.Bool after cmd.Wait() returns and makes exited() read that instead of cmd.ProcessState. It also adds a test which calls exited() while Wait() is running. It fails with -race before this fix and passes after it.
103 lines
2.5 KiB
Go
103 lines
2.5 KiB
Go
//go:build !plan9
|
|
|
|
package sftp
|
|
|
|
import (
|
|
"testing"
|
|
"time"
|
|
|
|
"github.com/rclone/rclone/fs"
|
|
"github.com/stretchr/testify/assert"
|
|
)
|
|
|
|
// TestSSHExternalWaitMultipleCalls verifies that calling Wait() multiple times
|
|
// doesn't cause zombie processes
|
|
func TestSSHExternalWaitMultipleCalls(t *testing.T) {
|
|
// Create a minimal Fs object for testing
|
|
opt := &Options{
|
|
SSH: fs.SpaceSepList{"echo", "test"},
|
|
}
|
|
|
|
f := &Fs{
|
|
opt: *opt,
|
|
}
|
|
|
|
// Create a new SSH session
|
|
session := f.newSSHSessionExternal()
|
|
|
|
// Start a simple command that exits quickly
|
|
err := session.Start("exit 0")
|
|
assert.NoError(t, err)
|
|
|
|
// Give the command time to complete
|
|
time.Sleep(100 * time.Millisecond)
|
|
|
|
// Call Wait() multiple times - this should not cause issues
|
|
err1 := session.Wait()
|
|
err2 := session.Wait()
|
|
err3 := session.Wait()
|
|
|
|
// All calls should return the same result (no error in this case)
|
|
assert.NoError(t, err1)
|
|
assert.NoError(t, err2)
|
|
assert.NoError(t, err3)
|
|
|
|
// Verify the process has exited
|
|
assert.True(t, session.exited())
|
|
}
|
|
|
|
// TestSSHExternalCloseMultipleCalls verifies that calling Close() multiple times
|
|
// followed by Wait() calls doesn't cause zombie processes
|
|
func TestSSHExternalCloseMultipleCalls(t *testing.T) {
|
|
// Create a minimal Fs object for testing
|
|
opt := &Options{
|
|
SSH: fs.SpaceSepList{"sleep", "10"},
|
|
}
|
|
|
|
f := &Fs{
|
|
opt: *opt,
|
|
}
|
|
|
|
// Create a new SSH session
|
|
session := f.newSSHSessionExternal()
|
|
|
|
// Start a long-running command
|
|
err := session.Start("sleep 10")
|
|
if err != nil {
|
|
t.Skip("Cannot start sleep command:", err)
|
|
}
|
|
|
|
// Close should cancel and wait for the process
|
|
_ = session.Close()
|
|
|
|
// Additional Wait() calls should return the same error
|
|
err2 := session.Wait()
|
|
err3 := session.Wait()
|
|
|
|
// All should complete without panicking
|
|
// err1 could be nil or an error depending on how the process was killed
|
|
// err2 and err3 should be the same
|
|
assert.Equal(t, err2, err3, "Subsequent Wait() calls should return same result")
|
|
|
|
// Verify the process has exited
|
|
assert.True(t, session.exited())
|
|
}
|
|
|
|
// TestSSHExternalExitedWhileWaiting verifies that exited() can be called
|
|
// while another goroutine is waiting for the process. Run with -race.
|
|
func TestSSHExternalExitedWhileWaiting(t *testing.T) {
|
|
f := &Fs{
|
|
opt: Options{
|
|
SSH: fs.SpaceSepList{"echo", "test"},
|
|
},
|
|
}
|
|
session := f.newSSHSessionExternal()
|
|
assert.NoError(t, session.Start("exit 0"))
|
|
|
|
go func() {
|
|
_ = session.Wait()
|
|
}()
|
|
|
|
assert.Eventually(t, session.exited, 10*time.Second, time.Millisecond)
|
|
}
|