io.Copy and Writer

[Subramanian Sridharan]

I had an issue where io.Copy did not copy the entire file contents.

My code was something like:

fReader := bufio.NewReader(sourcef)

fWriter := bufio.NewWriter(destf)

n, err := io.Copy(fWriter, fReader)

if err != nil {

fmt.Println("Error while copying:", err)

return

}

fmt.Println("Copied", n, "bytes.")

After searching a bit, I learnt that Writers should be flushed to ensure that all of their contents has been written to the destination.

I modified my code as:

fReader := bufio.NewReader(sourcef)

fWriter := bufio.NewWriter(destf)

n, err := io.Copy(fWriter, fReader)

if err != nil {

fmt.Println("Error while copying:", err)

return

}

// Flush the writer to write remaining bytes in the buffer.

err = fWriter.Flush()

if err != nil {

fmt.Println("Error while flushing writer:", err)

} else {

fmt.Println("Flushed writer.")

}

fmt.Println("Copied", n, "bytes.")

Now the entire file contents were copied successfully.

But I have a couple questions in mind:

1. On searching, I found out in Java, the writers are automatically flushed when the corresponding file descriptor is closed. Even though I had deferred close statements on my source and destination files, the writer was not flushed. Why is the behaviour not present in Go?
2. I saw other usages of io.Copy in my organization's codebase and they have used file descriptors directly in place of readers and writers. Hence no need of flushing. Is there any particular reason why I should use readers and writers and not the file descriptors directly?

TIA.

[pierre...@gmail.com]

Hello,

Indeed, you need to flush the bufio.Writer for the remaining data to be sent accross.

Usually though, you do not need to buffer the input or output at all.

In which case, the code is straight forward:

https://play.golang.org/p/F1tMKdsapyX

If you do need to buffer though, I would do something along the lines of:

https://play.golang.org/p/e5xFL_nUdp5

or even

https://play.golang.org/p/3I1J_4zOKL4

I would stringly encourage you to *use* interfaces such as the io.Reader and io.Writer and *not* file descriptors. You will find that they are very easy to work with and make your code much easier to read, test and maintain in the long run.

HTH

[Jan Mercl]

On Tue, May 21, 2019 at 9:44 AM <pierre...@gmail.com> wrote:

> Usually though, you do not need to buffer the input or output at all.

If I/O is performed in chunks comparable in size to what
bufio.{Reader,Writer} uses then ok. When I/O is done in small chunks,
then using bufio should improve performance.

The question left is what the "usual" scenario is.

[Robert Engels]

You do not need to flush if you call close. Close will flush. There is some other bug in your code I believe.

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAA40n-WsWkLtnP19DUwHNSrjDNBncbVepqNxVj%3DUF6MFV8ARLA%40mail.gmail.com.
> For more options, visit https://groups.google.com/d/optout.

[Subramanian Sridharan]

I don't think so.

When I close the file and don't explicitly flush:

package main

import (

"bufio"

"fmt"

"io"

"os"

)

func main() {

sourceFilename := "MozillaFirefox-66.0.5-741.4.x86_64.rpm"

sourcef, err := os.Open(sourceFilename)

if err != nil {

fmt.Println("Error while opening source file:", err)

return

}

defer sourcef.Close()

destinationFilename := "CopiedMozillaFirefox-66.0.5-741.4.x86_64.rpm"

os.Create(destinationFilename)

destf, err := os.OpenFile(destinationFilename, os.O_APPEND|os.O_WRONLY, os.ModeAppend)

if err != nil {

fmt.Println("Error while opening destination file:", err)

return

}

defer func() {

fmt.Println("Closing file.")

destf.Close()

}()

fReader := bufio.NewReader(sourcef)

fWriter := bufio.NewWriter(destf)

n, err := io.Copy(fWriter, fReader)

if err != nil {

fmt.Println("Error while copying:", err)

return

}

fmt.Println("Copied", n, "bytes.")

}

Output:

➜  throttle-and-resume-download go run copyTest.go                                                                      

Copied 46061104 bytes.

Closing file.

➜  throttle-and-resume-download diff MozillaFirefox-66.0.5-741.4.x86_64.rpm CopiedMozillaFirefox-66.0.5-741.4.x86_64.rpm

Binary files MozillaFirefox-66.0.5-741.4.x86_64.rpm and CopiedMozillaFirefox-66.0.5-741.4.x86_64.rpm differ

➜  throttle-and-resume-download echo $?

1

When I explicitly flush:

package main

import (

"bufio"

"fmt"

"io"

"os"

)

func main() {

sourceFilename := "MozillaFirefox-66.0.5-741.4.x86_64.rpm"

sourcef, err := os.Open(sourceFilename)

if err != nil {

fmt.Println("Error while opening source file:", err)

return

}

defer sourcef.Close()

destinationFilename := "CopiedMozillaFirefox-66.0.5-741.4.x86_64.rpm"

os.Create(destinationFilename)

destf, err := os.OpenFile(destinationFilename, os.O_APPEND|os.O_WRONLY, os.ModeAppend)

if err != nil {

fmt.Println("Error while opening destination file:", err)

return

}

defer func() {

fmt.Println("Closing file.")

destf.Close()

}()

fReader := bufio.NewReader(sourcef)

fWriter := bufio.NewWriter(destf)

n, err := io.Copy(fWriter, fReader)

if err != nil {

fmt.Println("Error while copying:", err)

return

}

// Flush writer to ensure all contents are written to file

err = fWriter.Flush()

if err != nil {

fmt.Println("Error while flushing writer:", err)

return

} else {

fmt.Println("Flushed writer.")

}

fmt.Println("Copied", n, "bytes.")

}

Output:

➜  throttle-and-resume-download go run copyTest.go                                                                      

Flushed writer.

Copied 46061104 bytes.

Closing file.

➜  throttle-and-resume-download diff MozillaFirefox-66.0.5-741.4.x86_64.rpm CopiedMozillaFirefox-66.0.5-741.4.x86_64.rpm

➜  throttle-and-resume-download echo $?                                                                                 

0

On Tuesday, May 21, 2019 at 6:39:08 PM UTC+5:30, Robert Engels wrote:

You do not need to flush if you call close. Close will flush. There is some other bug in your code I believe.

> On May 21, 2019, at 3:01 AM, Jan Mercl <0xj...@gmail.com> wrote:
>
>> On Tue, May 21, 2019 at 9:44 AM <pierr...@gmail.com> wrote:
>>
>> Usually though, you do not need to buffer the input or output at all.
>
> If I/O is performed in chunks comparable in size to what
> bufio.{Reader,Writer} uses then ok. When I/O is done in small chunks,
> then using bufio should improve performance.
>
> The question left is what the "usual" scenario is.
>
> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to golan...@googlegroups.com.

[Robert Engels]

Sorry, per docs, the client should call the Flush method to guarantee all data has been forwarded to the underlying io.Writer.

Not sure why the Write doesn’t implement Closer. That’s fairly non standard way of doing buffered io...

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/f4541ff6-f94c-4a99-a403-797e5f5b566b%40googlegroups.com.

[howar...@gmail.com]

Excuse me if I am misunderstanding something - but it certainly looks to me like you are not at any point closing the bufio.Writer (because it is not a WriteCloser and does not support Close function). That is why you need to flush it! What you are closing is the underlying file/stream/WriteCloser, but it is the bufio.Writer that has the data in its buffer, *not* the file. This is not the same thing as flushing a file descriptor, which happens on close.

"After all data has been written, the client should call the Flush method to guarantee all data has been forwarded to the underlying io.Writer."

IF the bufio.Writer had been implemented as a WriteCloser, then closing it would probably flush as well - here is a discussion where they talk about how that could be implemented: https://stackoverflow.com/questions/43115699/how-to-get-a-bufio-writer-that-implements-io-writecloser

Howard

[Robert Engels]

That’s what I was saying, I incorrectly thought that the Writer was a WriterCloser.

Seems it should be with a noop if the contained Writer is not a WriterCloser. This would be pretty much how every other platform does buffered IO. 

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.

To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/24d5242a-3e5a-4b83-a6f2-1ca7e29cfe0e%40googlegroups.com.

[roger peppe]

Probably it's not a Closer because that would imply that Close would close its underlying writer too, but then it would need to take a WriteCloser which would make it less general.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/D61080CC-6271-4782-88A8-9213A5C12A95%40ix.netcom.com.

[Robert Engels]

That’s a good case of dynamic type checking and good documentation

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAJhgacidQJLbjQ16BBDLOQ57q5BAbqJvu1Y57-itoAV9DtPDLQ%40mail.gmail.com.

[Robert Engels]

The more I think about this, the more I believe that this is a horrible situation that breaks all rules of encapsulation. This requires the creator to maintain references to the underlying Writer. It also requires the creator to be the sole decider of when the Writer is closed, or the creator must pass the concrete class to subordinates so they can close, or the subordinates must do type casting. 

The bufio.Writer is badly broken and should be changed to declare Close and pass this to the contained Writer if it is a WriterCloser. 

[Ian Lance Taylor]

On Wed, May 22, 2019 at 9:02 AM Robert Engels <ren...@ix.netcom.com> wrote:
>
> The more I think about this, the more I believe that this is a horrible situation that breaks all rules of encapsulation. This requires the creator to maintain references to the underlying Writer. It also requires the creator to be the sole decider of when the Writer is closed, or the creator must pass the concrete class to subordinates so they can close, or the subordinates must do type casting.
>
> The bufio.Writer is badly broken and should be changed to declare Close and pass this to the contained Writer if it is a WriterCloser.

What to open a proposal for that? https://golang.org/s/proposal.

Ian

[Jan Mercl]

On Wed, May 22, 2019 at 3:15 PM Ian Lance Taylor <ia...@golang.org> wrote:

> > The bufio.Writer is badly broken and should be changed to declare Close and pass this to the contained Writer if it is a WriterCloser.
>
> What to open a proposal for that? https://golang.org/s/proposal.

As I cannot vote on proposals without having a Microsoft owned
account, please let me put my down vote on such proposal preemptively
here.

[howar...@gmail.com]

Except that as the example I gave showed, it is fairly trivial to wrap the bufio.Writer into a WriteCloser that retains that reference and performs the pass-through Close as expected. So if you have a situation that needs a WriteCloser, you can manage that. Meanwhile, the bufio classes provide valuable functionality to Writers and Readers, whether or not they have a Close function, at the cost of a simple requirement to call Flush after writes finish.

The current implementation *is* the more general one. So perhaps what would be better would be to add a bufio.WriteCloser that requires a WriteCloser to wrap, so that we don't have to implement one of our own in those cases.

Howard

[Robert Engels]

Now that’s a good idea ! Hopefully sometime today. :)

[Robert Engels]

The very fact that you downvote a proposal without reading it says a lot.

[roger peppe]

I have to say that I must prefer APIs that do not take arbitrary advantage of dynamically discovered methods. It makes things less clear, and adding a wrapper to flush and then close the underlying writer is not hard. In fact the idiom for doing so is probably the reason why it's not feasible to add a Close method to `bufio.Writer`. If you do that, then you run the risk of breaking current code that adds its own Close method:

https://play.golang.org/p/xidNo2Z_OYB

This would definitely break some code that I've written.

   cheers,

      rog.

[Robert Engels]

I don’t believe your example is correct. In order for a current wrapper to call Flush on Close on the bufio.Writer the Close would need to be defined on the Wrapper struct so there would be no name collision. 

I think it is best to see the proposal before commenting. I plan on addressing any code breaking issues - if there were obvious ones I wouldn’t be for the proposal either.