From 51dad1d1301166aaf0d08a78ca67bc01f1168f42 Mon Sep 17 00:00:00 2001 From: Klaus Post Date: Fri, 8 Jan 2021 10:12:26 -0800 Subject: [PATCH] Fix missing GetObjectNInfo Closure (#11243) Review for missing Close of returned value from `GetObjectNInfo`. This was often obscured by the stuff that auto-unlocks when reaching EOF. --- cmd/bucket-lifecycle.go | 4 ++-- cmd/bucket-replication.go | 2 ++ cmd/data-usage-cache.go | 1 + pkg/bucket/bandwidth/reader.go | 16 ++++++++-------- 4 files changed, 13 insertions(+), 10 deletions(-) diff --git a/cmd/bucket-lifecycle.go b/cmd/bucket-lifecycle.go index 4e4d97c9e..93b1b9c08 100644 --- a/cmd/bucket-lifecycle.go +++ b/cmd/bucket-lifecycle.go @@ -341,13 +341,13 @@ func transitionObject(ctx context.Context, objectAPI ObjectLayer, objInfo Object } oi := gr.ObjInfo if oi.TransitionStatus == lifecycle.TransitionComplete { - gr.Close() // make sure to avoid leaks. + gr.Close() return nil } putOpts := putTransitionOpts(oi) if _, err = tgt.PutObject(ctx, arn.Bucket, oi.Name, gr, oi.Size, "", "", putOpts); err != nil { - gr.Close() // make sure to avoid leaks. + gr.Close() return err } gr.Close() diff --git a/cmd/bucket-replication.go b/cmd/bucket-replication.go index 59ae32ae4..719b762b8 100644 --- a/cmd/bucket-replication.go +++ b/cmd/bucket-replication.go @@ -471,6 +471,8 @@ func replicateObject(ctx context.Context, objInfo ObjectInfo, objectAPI ObjectLa for k, v := range putOpts.Header() { headerSize += len(k) + len(v) } + + // r takes over closing gr. r := bandwidth.NewMonitoredReader(ctx, globalBucketMonitor, objInfo.Bucket, objInfo.Name, gr, headerSize, b, target.BandwidthLimit) _, err = tgt.PutObject(ctx, dest.Bucket, object, r, size, "", "", putOpts) if err != nil { diff --git a/cmd/data-usage-cache.go b/cmd/data-usage-cache.go index c1591192a..d7483ceb2 100644 --- a/cmd/data-usage-cache.go +++ b/cmd/data-usage-cache.go @@ -478,6 +478,7 @@ func (d *dataUsageCache) load(ctx context.Context, store objectIO, name string) *d = dataUsageCache{} return nil } + defer r.Close() if err := d.deserialize(r); err != nil { *d = dataUsageCache{} logger.LogOnceIf(ctx, err, err.Error()) diff --git a/pkg/bucket/bandwidth/reader.go b/pkg/bucket/bandwidth/reader.go index 9c85f0406..468ba26da 100644 --- a/pkg/bucket/bandwidth/reader.go +++ b/pkg/bucket/bandwidth/reader.go @@ -28,7 +28,7 @@ type MonitoredReader struct { bucket string // Token to track bucket bucketMeasurement *bucketMeasurement // bucket measurement object object string // Token to track object - reader io.Reader // Reader to wrap + reader io.ReadCloser // Reader to wrap lastStop time.Time // Last timestamp for a measurement headerSize int // Size of the header not captured by reader throttle *throttle // throttle the rate at which replication occur @@ -36,8 +36,9 @@ type MonitoredReader struct { closed bool // Reader is closed } -// NewMonitoredReader returns a io.ReadCloser that reports bandwidth details -func NewMonitoredReader(ctx context.Context, monitor *Monitor, bucket string, object string, reader io.Reader, headerSize int, bandwidthBytesPerSecond int64, clusterBandwidth int64) *MonitoredReader { +// NewMonitoredReader returns a io.ReadCloser that reports bandwidth details. +// The supplied reader will be closed. +func NewMonitoredReader(ctx context.Context, monitor *Monitor, bucket string, object string, reader io.ReadCloser, headerSize int, bandwidthBytesPerSecond int64, clusterBandwidth int64) *MonitoredReader { timeNow := time.Now() b := monitor.track(bucket, object, timeNow) return &MonitoredReader{ @@ -77,10 +78,9 @@ func (m *MonitoredReader) Read(p []byte) (n int, err error) { // Close stops tracking the io func (m *MonitoredReader) Close() error { - rc, ok := m.reader.(io.ReadCloser) - m.closed = true - if ok { - return rc.Close() + if m.closed { + return nil } - return nil + m.closed = true + return m.reader.Close() }