fix: always check error upon w.Close() in Write() (#18111)

not checking w.Close() can prematurely make us
think that the w.Write() actually succeeded, apparently
Write() may or may not return an error but sometimes
only during a Close() call to the fd we may see the
error from Write() propagate.

Fdatasync(w) on the FD would return an error requiring
Close() error handling is less of a concern, however it may
happen such that fdatasync() did not return an error, where
as Close() would.
This commit is contained in:
Harshavardhana 2023-09-26 11:04:00 -07:00 committed by GitHub
parent 22ee678136
commit cdeab19673
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
1 changed files with 28 additions and 4 deletions

View File

@ -1945,7 +1945,6 @@ func (s *xlStorage) writeAllDirect(ctx context.Context, filePath string, fileSiz
if err != nil {
return osErrToFileErr(err)
}
defer w.Close()
var bufp *[]byte
switch {
@ -1968,17 +1967,33 @@ func (s *xlStorage) writeAllDirect(ctx context.Context, filePath string, fileSiz
written, err = io.CopyBuffer(diskHealthWriter(ctx, w), r, *bufp)
}
if err != nil {
w.Close()
return err
}
if written < fileSize && fileSize >= 0 {
w.Close()
return errLessData
} else if written > fileSize && fileSize >= 0 {
w.Close()
return errMoreData
}
// Only interested in flushing the size_t not mtime/atime
return Fdatasync(w)
if err = Fdatasync(w); err != nil {
w.Close()
return err
}
// Dealing with error returns from close() - 'man 2 close'
//
// A careful programmer will check the return value of close(), since it is quite possible that
// errors on a previous write(2) operation are reported only on the final close() that releases
// the open file descriptor.
//
// Failing to check the return value when closing a file may lead to silent loss of data.
// This can especially be observed with NFS and with disk quota.
return w.Close()
}
func (s *xlStorage) writeAll(ctx context.Context, volume string, path string, b []byte, sync bool) (err error) {
@ -2016,18 +2031,27 @@ func (s *xlStorage) writeAll(ctx context.Context, volume string, path string, b
if err != nil {
return err
}
defer w.Close()
n, err := w.Write(b)
if err != nil {
w.Close()
return err
}
if n != len(b) {
w.Close()
return io.ErrShortWrite
}
return nil
// Dealing with error returns from close() - 'man 2 close'
//
// A careful programmer will check the return value of close(), since it is quite possible that
// errors on a previous write(2) operation are reported only on the final close() that releases
// the open file descriptor.
//
// Failing to check the return value when closing a file may lead to silent loss of data.
// This can especially be observed with NFS and with disk quota.
return w.Close()
}
func (s *xlStorage) WriteAll(ctx context.Context, volume string, path string, b []byte) (err error) {