From 4223ebab8dbac847a1767fdd925028432fcc9812 Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Wed, 7 Apr 2021 13:29:27 -0700 Subject: [PATCH] fix: remove auto-close GetObjectReader (#12009) locks can get relinquished when Read() sees io.EOF leading to prematurely closing of the readers concurrent writes on the same object can have undesired consequences here when these locks are relinquished. --- cmd/bucket-lifecycle.go | 2 +- cmd/object-api-utils.go | 8 +------- pkg/s3select/select.go | 12 +++++++++++- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/cmd/bucket-lifecycle.go b/cmd/bucket-lifecycle.go index db74658b0..666f650fa 100644 --- a/cmd/bucket-lifecycle.go +++ b/cmd/bucket-lifecycle.go @@ -530,7 +530,7 @@ type SelectParameters struct { // IsEmpty returns true if no select parameters set func (sp *SelectParameters) IsEmpty() bool { - return sp == nil || sp.S3Select == s3select.S3Select{} + return sp == nil } var ( diff --git a/cmd/object-api-utils.go b/cmd/object-api-utils.go index c2c7a38f6..ffc7c7386 100644 --- a/cmd/object-api-utils.go +++ b/cmd/object-api-utils.go @@ -817,13 +817,7 @@ func (g *GetObjectReader) Close() error { // Read - to implement Reader interface. func (g *GetObjectReader) Read(p []byte) (n int, err error) { - n, err = g.pReader.Read(p) - if err != nil { - // Calling code may not Close() in case of error, so - // we ensure it. - g.Close() - } - return + return g.pReader.Read(p) } //SealMD5CurrFn seals md5sum with object encryption key and returns sealed diff --git a/pkg/s3select/select.go b/pkg/s3select/select.go index cbf48cbf9..326547962 100644 --- a/pkg/s3select/select.go +++ b/pkg/s3select/select.go @@ -1,5 +1,5 @@ /* - * MinIO Cloud Storage, (C) 2019 MinIO, Inc. + * MinIO Cloud Storage, (C) 2019-2021 MinIO, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -216,6 +216,7 @@ type S3Select struct { statement *sql.SelectStatement progressReader *progressReader recordReader recordReader + close func() error } var ( @@ -311,6 +312,7 @@ func (s3Select *S3Select) Open(getReader func(offset, length int64) (io.ReadClos } return err } + s3Select.close = rc.Close return nil case jsonFormat: rc, err := getReader(0, -1) @@ -333,6 +335,8 @@ func (s3Select *S3Select) Open(getReader func(offset, length int64) (io.ReadClos } else { s3Select.recordReader = json.NewReader(s3Select.progressReader, &s3Select.Input.JSONArgs) } + + s3Select.close = rc.Close return nil case parquetFormat: if !strings.EqualFold(os.Getenv("MINIO_API_SELECT_PARQUET"), "on") { @@ -396,6 +400,12 @@ func (s3Select *S3Select) marshal(buf *bytes.Buffer, record sql.Record) error { // Evaluate - filters and sends records read from opened reader as per select statement to http response writer. func (s3Select *S3Select) Evaluate(w http.ResponseWriter) { + defer func() { + if s3Select.close != nil { + s3Select.close() + } + }() + getProgressFunc := s3Select.getProgress if !s3Select.Progress.Enabled { getProgressFunc = nil