Add `io.Closer` guidelines (#29387)

Co-authored-by: Yarden Shoham <git@yardenshoham.com>

diff --git a/docs/content/contributing/guidelines-backend.en-us.md b/docs/content/contributing/guidelines-backend.en-us.md
index 084b3886e84e588473f8c3920cee48bb87a37517..3159a5ff7d14564fa3e225427e5c484970ec16e5 100644 (file)
--- a/docs/content/contributing/guidelines-backend.en-us.md
+++ b/docs/content/contributing/guidelines-backend.en-us.md
@@ -101,6 +101,10 @@ i.e. `services/user`, `models/repository`.
 Since there are some packages which use the same package name, it is possible that you find packages like `modules/user`, `models/user`, and `services/user`. When these packages are imported in one Go file, it's difficult to know which package we are using and if it's a variable name or an import name. So, we always recommend to use import aliases. To differ from package variables which are commonly in camelCase, just use **snake_case** for import aliases.
 i.e. `import user_service "code.gitea.io/gitea/services/user"`
 
+### Implementing `io.Closer`
+
+If a type implements `io.Closer`, calling `Close` multiple times must not fail or `panic` but return an error or `nil`.
+
 ### Important Gotchas
 
 - Never write `x.Update(exemplar)` without an explicit `WHERE` clause:

diff --git a/modules/git/blame.go b/modules/git/blame.go
index 64095a218a31c9d67ea5e6c0f580e022a96f1d81..69e1b08f93b462672f0fbfb411ef8634097bf30d 100644 (file)
--- a/modules/git/blame.go
+++ b/modules/git/blame.go
@@ -115,6 +115,10 @@ func (r *BlameReader) NextPart() (*BlamePart, error) {
 
 // Close BlameReader - don't run NextPart after invoking that
 func (r *BlameReader) Close() error {
+       if r.bufferedReader == nil {
+               return nil
+       }
+
        err := <-r.done
        r.bufferedReader = nil
        _ = r.reader.Close()

diff --git a/modules/git/repo_base_gogit.go b/modules/git/repo_base_gogit.go
index 9270bb70f07342b974645b3b83516485dfee1a3a..3ca5eb36c6eded1e0d42ae3b224c7ecd7b62ef9c 100644 (file)
--- a/modules/git/repo_base_gogit.go
+++ b/modules/git/repo_base_gogit.go
@@ -88,16 +88,17 @@ func OpenRepository(ctx context.Context, repoPath string) (*Repository, error) {
 }
 
 // Close this repository, in particular close the underlying gogitStorage if this is not nil
-func (repo *Repository) Close() (err error) {
+func (repo *Repository) Close() error {
        if repo == nil || repo.gogitStorage == nil {
-               return
+               return nil
        }
        if err := repo.gogitStorage.Close(); err != nil {
                gitealog.Error("Error closing storage: %v", err)
        }
+       repo.gogitStorage = nil
        repo.LastCommitCache = nil
        repo.tagCache = nil
-       return
+       return nil
 }
 
 // GoGitRepo gets the go-git repo representation

diff --git a/modules/git/repo_base_nogogit.go b/modules/git/repo_base_nogogit.go
index 8c6eae5897eb09cabd4471386af1b4fb57cf2b0f..86b6a935677536d7a5aac7dca1c16acc9f1c1714 100644 (file)
--- a/modules/git/repo_base_nogogit.go
+++ b/modules/git/repo_base_nogogit.go
@@ -103,7 +103,7 @@ func (repo *Repository) CatFileBatchCheck(ctx context.Context) (WriteCloserError
        }
 }
 
-func (repo *Repository) Close() (err error) {
+func (repo *Repository) Close() error {
        if repo == nil {
                return nil
        }
@@ -123,5 +123,5 @@ func (repo *Repository) Close() (err error) {
        }
        repo.LastCommitCache = nil
        repo.tagCache = nil
-       return err
+       return nil
 }

diff --git a/modules/indexer/internal/bleve/indexer.go b/modules/indexer/internal/bleve/indexer.go
index ce06b5afcb7885490be6a8d539ad135d6a6d0edc..01e53ca636036fac31693de23b8722a609c62fae 100644 (file)
--- a/modules/indexer/internal/bleve/indexer.go
+++ b/modules/indexer/internal/bleve/indexer.go
@@ -92,7 +92,7 @@ func (i *Indexer) Ping(_ context.Context) error {
 }
 
 func (i *Indexer) Close() {
-       if i == nil {
+       if i == nil || i.Indexer == nil {
                return
        }
 

diff --git a/modules/indexer/internal/meilisearch/indexer.go b/modules/indexer/internal/meilisearch/indexer.go
index b037249d43210c6e21ff3d25f0b895e63b1989b5..f4004849c1d05749fbf992e1fd72efe6e3d6a2cf 100644 (file)
--- a/modules/indexer/internal/meilisearch/indexer.go
+++ b/modules/indexer/internal/meilisearch/indexer.go
@@ -87,8 +87,5 @@ func (i *Indexer) Close() {
        if i == nil {
                return
        }
-       if i.Client == nil {
-               return
-       }
        i.Client = nil
 }

diff --git a/modules/util/filebuffer/file_backed_buffer.go b/modules/util/filebuffer/file_backed_buffer.go
index 6b07bd04138640cbd79bde91c531565014268baf..739543e2977efd12bd5c61e9313b3db2e08a38e7 100644 (file)
--- a/modules/util/filebuffer/file_backed_buffer.go
+++ b/modules/util/filebuffer/file_backed_buffer.go
@@ -149,6 +149,7 @@ func (b *FileBackedBuffer) Close() error {
        if b.file != nil {
                err := b.file.Close()
                os.Remove(b.file.Name())
+               b.file = nil
                return err
        }
        return nil