Skip to content

close file double time sometimes cause copy problem wh #572

@lindmin

Description

@lindmin

Hello,

version of go : 1.20.5
version of lib sftp : v1.13.6
version of sftp server: OpenSSH_8.9p1 Ubuntu-3ubuntu0.6

I sometime experiment a bug when my code close 2 times the same sftp.File ref in concurrent situation

here the code to reproduce :

main.go

package main

import (
	"bytes"
	"fmt"
	"net"
	"os"
	"sync"

	"github.com/pkg/sftp"
	"golang.org/x/crypto/ssh"
)

func main() {
	connSSH, errSSH := ssh.Dial("tcp", net.JoinHostPort("localhost", "22"), &ssh.ClientConfig{
		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
		User:            "USER",
		Auth: []ssh.AuthMethod{
			ssh.Password("PASSWORD"),
		},
	})
	if errSSH != nil {
		panic(errSSH)
	}
	client, errSftp := sftp.NewClient(connSSH, sftp.UseConcurrentReads(true), sftp.UseConcurrentWrites(true), sftp.UseFstat(true))
	if errSftp != nil {
		panic((errSftp))
	}
	if errSftp != nil {
		panic(errSftp.Error())
	}
	input := make(chan int, 1000)

	for i := 1; i < 1000; i++ {
		input <- i
	}

	wait := &sync.WaitGroup{}
	for i := 1; i <= 1000; i++ {
		wait.Add(1)
		go func(wait *sync.WaitGroup) {
			defer wait.Done()
			for {
				idx, ok := <-input
				if !ok {
					return
				}
				filename := fmt.Sprintf("/tmp/file-%d.dat", idx)
				err := copy(client, filename, "small content")
				if err != nil {
					fmt.Println(err.Error())
				}
			}
		}(wait)
	}
	close(input)
	wait.Wait()

}

func copy(client *sftp.Client, filename string, content string) error {
	file, errCreate := client.OpenFile(filename, os.O_WRONLY|os.O_CREATE|os.O_TRUNC)
	if errCreate != nil {
		return errCreate
	}
	_, errCopy := file.ReadFrom(bytes.NewBufferString(content))
	if errCopy != nil {					
               fmt.Printf("💣💣💣💣💣 COPY: %s", err.Error())
		//clean it
		client.Remove(filename)
		return errCopy
	}
	defer file.Close()
	//force double close
	file.Close()
	return nil
}

go.mod

module golang_sftp_fd_null

go 1.20

require (
	github.com/pkg/sftp v1.13.6
	golang.org/x/crypto v0.9.0
)

require github.com/stretchr/testify v1.8.1 // indirect

require (
	github.com/kr/fs v0.1.0 // indirect
	golang.org/x/sys v0.8.0 // indirect
)

When i launched this code, sometime i got 1 or 2 SSH_FX_FAILURE on the ReadFrom method .

output of my code :

💣💣💣💣💣 COPY: sftp: "Failure" (SSH_FX_FAILURE)

On server side, I can see that type of logs:

Jan 11 15:27:04 BVK-book sftp-server[185806]: debug1: request 2162: write "(null)" (handle -1) off 0 len 14
Jan 11 15:27:04 BVK-book sftp-server[185806]: debug3: request 2162: sent status 4
Jan 11 15:27:04 BVK-book sftp-server[185806]: sent status Failure

Now, if I replace

	defer file.Close()
	//force double close
	file.Close()

with

        alreadyClosed:=false
	defer func(){
       if !alreadyClosed{
           file.Close()        
        }
        }
	//force double close
	file.Close()

I cant reproduce the error.

In the Close method documentation, it is mention that Close closes the File, rendering it unusable for I/O..
Is-it really the case ?

Does a PR that add a flag check "alreadyClose" in the Close method before send a close request to sftp server is a great idea ?

thank you.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions