Client stuck when trying to read with io.CopyN() in golang

Question

I am trying to make TCP server for transferring files. I am suing io.CopyN for reading and writing. From server side, I am sending files to client so from server side, it sends perfectly all bytes but Client side after reading a couple of 1000000 bytes it stuck. sometimes it works fine and sometimes it gets stuck. I am using 300 MB pdf to test. Any help, code, and output is like below.

server

    package main

    import (
        "fmt"
        "io"
        "log"
        "net"
        "os"
        "strconv"
        "strings"
    )

    func main() {

        ls, err := net.Listen("tcp", ":1234")

        errFunc(err)

        defer ls.Close()

        conn, _ := ls.Accept()

        defer conn.Close()

        for {

            file, err := os.Open(strings.TrimSpace("./" + "Mag" + ".pdf"))

            errFunc(err)

            defer file.Close()

            fileInfo, err := file.Stat()

            errFunc(err)

            size := fileInfo.Size()

            numberOfTime := size / 1000000

            leftByte := size - numberOfTime*1000000

            numberOfTimeString := strconv.Itoa(int(numberOfTime))
            leftByteString := strconv.Itoa(int(leftByte))

            fmt.Println("1000000 times : ", numberOfTimeString)

            fmt.Println("Left Bytes : ", leftByteString)

            _, err = fmt.Fprintf(conn, numberOfTimeString+"\n")

            errFunc(err)

            _, err = fmt.Fprintf(conn, leftByteString+"\n")

            errFunc(err)

            fileWriter := io.Writer(conn)

            for i := 0; i < int(numberOfTime); i++ {

                n, err := io.CopyN(conn, file, 1000000)

                if i >= 30 {
                    fmt.Println(err, n)
                }
            }

            n, err := io.CopyN(fileWriter, file, leftByte+1)

            if err == io.EOF {
                fmt.Println(err, n)
            }

            fmt.Printf("Succefully bytes sent : %v \n\n\n\n\n", n)

            file.Close()

        }

    }

    func errFunc(err error) {

        if err != nil {
            log.Fatal(err)
        }

    }

client

    package main

    import (
        "bufio"
        "fmt"
        "io"
        "net"
        "os"
        "os/signal"
        "strconv"
        "strings"
        "syscall"
    )

    func main() {

        c := make(chan os.Signal, 15)
        signal.Notify(c, syscall.SIGINT)

        go func() {

            for {
                s := <-c

                switch s {

                case syscall.SIGINT:
                    os.Exit(1)
                }

            }

        }()

        conn, _ := net.Dial("tcp", ":1234")

        defer conn.Close()

        connReadWrite := bufio.NewReader(io.Reader(conn))

        var i int
        var filename string

        for {

            i++

            nu := strconv.Itoa(i)

            filename = "image" + nu + ".pdf"

            file, err := os.Create(filename)

            defer file.Close()

            numberOfTimeString, err := connReadWrite.ReadString('\n')

            if err != nil {

                fmt.Println(err)
            }

            println("1000000 times :", numberOfTimeString)

            numberOfTimeString = strings.TrimSuffix(numberOfTimeString, "\n")

            numberOfTime, err := strconv.Atoi(numberOfTimeString)

            if err != nil {

                fmt.Println(err)
            }

            leftByteString, err := connReadWrite.ReadString('\n')

            if err != nil {

                println(err)
            }

            println("Left Bytes :", leftByteString)

            leftByteString = strings.TrimSuffix(leftByteString, "\n")

            leftByte, err := strconv.Atoi(leftByteString)

            if err != nil {

                panic(err)
            }

            fmt.Println("After convert in Num :", numberOfTime, leftByte)

            newFileWriter := io.Writer(file)
            newFileReader := io.Reader(conn)

            for i := 0; i < numberOfTime; i++ {

                n, err := io.CopyN(newFileWriter, newFileReader, 1000000)

                if i >= 30 {
                    errFun(err, n)
                }
            }

            n, err := io.CopyN(newFileWriter, newFileReader, int64(leftByte))

            errFun(err, n)

            fmt.Printf("sucessfully Transfered ---> \n\n\n\n\n\n")

        }

    }

    func errFun(err error, n int64) {

        if err == io.EOF {

            fmt.Println("End of file : ", n)
            return

        } else if n == 0 {

            fmt.Println("n is : ", n)
            return

        } else if err != nil {
            fmt.Println(err)
            return

        }

        fmt.Println(err, " : ", n)
    }

input/output

from server side first we are sending number of bytes it need to readand then client side it gets a number of bytes it needs to read and then I am sending the file and then it read. In the picture, I was able to send one-time second time it got stuck sometimes it stuck first time too.I am able to send number of byte from server side second time too but as you can see it don't read that numeber, it read something "%PDF..." and it even don't print "100000 times : " correctly it prints "%???00 times :" I just don’t understand this

enter image description here

Please come up with a _minimal_ example and please do not include screenshots. — Volker, Sep 08 '18 at 11:39
this is the minimal code required to run program completely... extra stuff is only opening files and close and err ... that's why it looks big but it is not :) — Parth Kabariya, Sep 08 '18 at 11:45
In a minimal example there should be no need to open files, you can debug it yourself if you cut out everything until you get to the bottom of the bug. That is what we call programming ;-) Try using a `bytes.Buffer` instead for example, or just send `[]byte`s directly. — gonutz, Sep 08 '18 at 11:57

score 2 · Accepted Answer · answered Sep 08 '18 at 21:25

I believe the issue is that you're using a bytes.Buffer in the client:

connReadWrite := bufio.NewReader(io.Reader(conn))

But you aren't using it later with the CopyN:

newFileWriter := io.Writer(file)
newFileReader := io.Reader(conn)
for i := 0; i < numberOfTime; i++ {
    _, err := io.CopyN(newFileWriter, newFileReader, 1000000)
    if err != nil {
        log.Fatalln(err)
    }
}

Using:

newFileWriter := io.Writer(file)

for i := 0; i < numberOfTime; i++ {
    _, err := io.CopyN(file, connReadWrite, 1000000)
    if err != nil {
        log.Fatalln(err)
    }
}

May fix it.

If you have control over the protocol you are using to send the file, I recommend doing something simpler. For example using the big-endian int64 length prefix.

Send:

func sendFile(name string, conn net.Conn) error {
    f, err := os.Open(name)
    if err != nil {
        return err
    }
    defer f.Close()

    fi, err := f.Stat()
    if err != nil {
        return err
    }
    sz := fi.Size()

    buf := bufio.NewWriter(conn)

    err = binary.Write(buf, binary.BigEndian, sz)
    if err != nil {
        return err
    }

    _, err = io.CopyN(buf, f, sz)
    if err != nil {
        return err
    }

    return buf.Flush()
}

Receive:

func recvFile(name string, conn net.Conn) error {
    f, err := os.Create(name)
    if err != nil {
        return err
    }
    defer f.Close()

    buf := bufio.NewReader(conn)
    var sz int64
    err = binary.Read(buf, binary.BigEndian, &sz)
    if err != nil {
        return err
    }

    _, err = io.CopyN(f, buf, sz)
    if err != nil {
        return err
    }

    return nil
}

I'd note that the essence of the problem (which comes up pretty often here) is failure to keep in mind that the buffered reader is free to read more data from its underlying reader than was requested by the user's code (actually, if it wasn't allowed to do that it could not do buffering after all). So once one wraps a reader into a buffered reader, one _must_ use only the wrapper to read the data. — kostix, Sep 09 '18 at 09:00
thank you very much... I have tried your first suggestion. I have changed to this _, err: = io.CopyN(file, connReadWrite, 1000000) and it worked I should keep it in mind :) — Parth Kabariya, Sep 10 '18 at 04:36
I should keep it in mind that if I wrap conn in one reader I don't create another one. but I have one question. after creating the first reader from conn I am reading two times so there are no more data and then I am creating new reader wrapper for conn again so what is the problem if you can explain I really want to know what's going behind it or if you can suggest some documents I appreciate if you explain it :) — Parth Kabariya, Sep 10 '18 at 05:02

Client stuck when trying to read with io.CopyN() in golang

server

client

input/output

1 Answers1