search

golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.43k stars 17.71k forks source link

x/build: numerous resource leaks reported by staticmajor #56191

Open odeke-em opened 2 years ago

odeke-em commented 2 years ago

Ran staticmajor within Orijtech Inc, and it produced this huge manifest of issues

/golang.org/x/build/autocertcache/autocertcache.go:49:3: leaking resource created on line 47
/golang.org/x/build/buildlet/grpcbuildlet.go:190:27: leaking resource
/golang.org/x/build/internal/coordinator/remote/ssh.go:338:3: leaking resource created on line 333
/golang.org/x/build/internal/coordinator/remote/ssh.go:341:3: leaking resource created on line 333
/golang.org/x/build/cmd/coordinator/coordinator.go:901:26: leaking resource
/golang.org/x/build/cmd/debugnewvm/debugnewvm.go:259:28: leaking resource
/golang.org/x/build/maintner/reclog/reclog.go:126:3: leaking resource created on line 120
/golang.org/x/build/maintner/reclog/reclog.go:130:3: leaking resource created on line 120
/golang.org/x/build/maintner/reclog/reclog.go:133:3: leaking resource created on line 120
/golang.org/x/build/maintner/netsource.go:657:3: leaking resource created on line 652
/golang.org/x/build/internal/gitauth/gitauth.go:35:28: leaking resource
/golang.org/x/build/tarutil/tarutil.go:75:4: leaking resource created on line 72
/golang.org/x/build/cmd/gomote/create.go:119:44: leaking resource
/golang.org/x/build/cmd/gomote/get.go:36:25: leaking resource
/golang.org/x/build/cmd/gomote/ls.go:40:25: leaking resource
/golang.org/x/build/cmd/gomote/ping.go:31:25: leaking resource
/golang.org/x/build/cmd/gomote/push.go:618:5: leaking resource created on line 605
/golang.org/x/build/cmd/gomote/push.go:627:4: leaking resource created on line 605
/golang.org/x/build/cmd/gomote/push.go:632:4: leaking resource created on line 605
/golang.org/x/build/cmd/gomote/push.go:637:4: leaking resource created on line 605
/golang.org/x/build/cmd/gomote/push.go:642:4: leaking resource created on line 605
/golang.org/x/build/cmd/gomote/push.go:57:32: leaking resource
/golang.org/x/build/cmd/gomote/put.go:387:27: leaking resource
/golang.org/x/build/cmd/gomote/put.go:54:25: leaking resource
/golang.org/x/build/cmd/gomote/put.go:209:32: leaking resource
/golang.org/x/build/cmd/gomote/put.go:263:25: leaking resource
/golang.org/x/build/cmd/gomote/rdp.go:44:3: leaking resource created on line 37
/golang.org/x/build/cmd/gomote/rdp.go:50:4: leaking resource created on line 37
/golang.org/x/build/cmd/gomote/rdp.go:42:23: leaking resource
/golang.org/x/build/cmd/gomote/rm.go:31:25: leaking resource
/golang.org/x/build/cmd/gomote/run.go:54:32: leaking resource
/golang.org/x/build/cmd/gomote/ssh.go:38:24: leaking resource
/golang.org/x/build/cmd/gomote/ssh.go:146:3: leaking resource created on line 141
/golang.org/x/build/cmd/gomote/ssh.go:149:3: leaking resource created on line 141
/golang.org/x/build/cmd/perfrun/perfrun.go:50:3: leaking resource created on line 42
/golang.org/x/build/cmd/perfrun/perfrun.go:58:4: leaking resource created on line 42
/golang.org/x/build/cmd/perfrun/perfrun.go:72:4: leaking resource created on line 42
/golang.org/x/build/cmd/perfrun/perfrun.go:77:4: leaking resource created on line 42
/golang.org/x/build/cmd/perfrun/perfrun.go:93:5: leaking resource created on line 42
/golang.org/x/build/cmd/perfrun/perfrun.go:98:5: leaking resource created on line 42
/golang.org/x/build/internal/task/buildrelease.go:59:3: leaking resource created on line 47
/golang.org/x/build/internal/task/buildrelease.go:62:3: leaking resource created on line 47
/golang.org/x/build/internal/task/buildrelease.go:431:4: leaking resource created on line 425
/golang.org/x/build/internal/task/buildrelease.go:436:4: leaking resource created on line 425
/golang.org/x/build/internal/task/buildrelease.go:451:4: leaking resource created on line 425
/golang.org/x/build/internal/task/buildrelease.go:457:4: leaking resource created on line 425
/golang.org/x/build/internal/task/tweet.go:542:3: leaking resource created on line 540
/golang.org/x/build/internal/task/tweet.go:544:3: leaking resource created on line 540
/golang.org/x/build/cmd/release/upload.go:191:3: leaking resource created on line 189
/golang.org/x/build/internal/relui/store.go:66:3: leaking resource created on line 60
/golang.org/x/build/maintner/maintnerd/gcslog/gcslog.go:450:4: leaking resource created on line 447
/golang.org/x/build/maintner/maintnerd/maintnerd.go:316:29: leaking resource
/golang.org/x/build/perfdata/client.go:176:3: leaking resource created on line 172
/golang.org/x/build/perfdata/db/db.go:57:3: leaking resource created on line 55
/golang.org/x/build/perfdata/db/db.go:60:3: leaking resource created on line 55
/golang.org/x/build/perfdata/db/db.go:232:15: leaking resource
/golang.org/x/build/perfdata/db/db.go:250:18: leaking resource
/golang.org/x/build/perfdata/fs/gcs/gcs.go:22:34: leaking resource

in which:

Kind FYI for @kirbyquerby @elias-orijtech @willpoint @jhusdero. I am sending a fix for this shortly!

The resource leaks are real as per this diff below, and all resource leaks except for the ones from x/perf which are fixed by this CL https://go-review.googlesource.com/c/perf/+/442656

```diff commit 62712d34e6600fec613fa936415d282eda19c6e0 Author: Emmanuel T Odeke Date: Thu Oct 13 00:49:58 2022 -0700 all: fix resource leaks reported by staticmajor Change-Id: Ib2bf2b9812e0a6ff4197754a670d6ad0e5f8416d diff --git a/autocertcache/autocertcache.go b/autocertcache/autocertcache.go index 2cbce707..a7974b59 100644 --- a/autocertcache/autocertcache.go +++ b/autocertcache/autocertcache.go @@ -45,10 +45,12 @@ func (c *gcsAutocertCache) Get(ctx context.Context, key string) ([]byte, error) func (c *gcsAutocertCache) Put(ctx context.Context, key string, data []byte) error { wr := c.gcs.Bucket(c.bucket).Object(key).NewWriter(ctx) - if _, err := wr.Write(data); err != nil { - return err + _, err := wr.Write(data) + cerr := wr.Close() + if err == nil { + err = cerr } - return wr.Close() + return err } func (c *gcsAutocertCache) Delete(ctx context.Context, key string) error { diff --git a/buildlet/grpcbuildlet.go b/buildlet/grpcbuildlet.go index 67c319bd..d0fc99dc 100644 --- a/buildlet/grpcbuildlet.go +++ b/buildlet/grpcbuildlet.go @@ -188,6 +188,8 @@ func (b *grpcBuildlet) upload(ctx context.Context, r io.Reader) (string, error) buf := new(bytes.Buffer) mw := multipart.NewWriter(buf) + defer mw.Close() + for k, v := range resp.Fields { if err := mw.WriteField(k, v); err != nil { return "", fmt.Errorf("unable to write field: %s", err) diff --git a/cmd/coordinator/coordinator.go b/cmd/coordinator/coordinator.go index df95301f..159a6d91 100644 --- a/cmd/coordinator/coordinator.go +++ b/cmd/coordinator/coordinator.go @@ -899,6 +899,8 @@ func findWorkLoop() { // do some new streaming gRPC call to maintnerd to subscribe // to new commits. ticker := time.NewTicker(15 * time.Second) + defer ticker.Stop() + // We wait for the ticker first, before looking for work, to // give findTryWork a head start. Because try work is more // important and the scheduler can't (yet?) preempt an diff --git a/cmd/debugnewvm/debugnewvm.go b/cmd/debugnewvm/debugnewvm.go index 8edf48f7..1114635e 100644 --- a/cmd/debugnewvm/debugnewvm.go +++ b/cmd/debugnewvm/debugnewvm.go @@ -260,6 +260,8 @@ func awsCredentialsFromSecrets() (string, string, error) { if err != nil { return "", "", fmt.Errorf("unable to create secret client: %w", err) } + defer c.Close() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() keyID, err := c.Retrieve(ctx, secret.NameAWSKeyID) diff --git a/cmd/gomote/create.go b/cmd/gomote/create.go index 8db5f31d..48d08120 100644 --- a/cmd/gomote/create.go +++ b/cmd/gomote/create.go @@ -128,6 +128,8 @@ func legacyCreate(args []string) error { if err != nil { return fmt.Errorf("failed to create buildlet: %v", err) } + defer client.Close() + fmt.Println(client.RemoteName()) return nil } diff --git a/cmd/gomote/get.go b/cmd/gomote/get.go index 98058c6d..cd02a7d7 100644 --- a/cmd/gomote/get.go +++ b/cmd/gomote/get.go @@ -37,6 +37,8 @@ func legacyGetTar(args []string) error { if err != nil { return err } + defer bc.Close() + tgz, err := bc.GetTar(context.Background(), dir) if err != nil { return err diff --git a/cmd/gomote/ls.go b/cmd/gomote/ls.go index ae7831b2..3b7d09c6 100644 --- a/cmd/gomote/ls.go +++ b/cmd/gomote/ls.go @@ -41,6 +41,8 @@ func legacyLs(args []string) error { if err != nil { return err } + defer bc.Close() + opts := buildlet.ListDirOpts{ Recursive: recursive, Digest: digest, diff --git a/cmd/gomote/ping.go b/cmd/gomote/ping.go index 879df5c8..2d312fa6 100644 --- a/cmd/gomote/ping.go +++ b/cmd/gomote/ping.go @@ -32,6 +32,8 @@ func legacyPing(args []string) error { if err != nil { return err } + defer bc.Close() + ctx := context.Background() wd, err := bc.WorkDir(ctx) if err != nil { diff --git a/cmd/gomote/push.go b/cmd/gomote/push.go index f914d0f3..196463aa 100644 --- a/cmd/gomote/push.go +++ b/cmd/gomote/push.go @@ -58,6 +58,7 @@ func legacyPush(args []string) error { if err != nil { return err } + defer bc.Close() haveGo14 := false remote := map[string]buildlet.DirEntry{} // keys like "src/make.bash" @@ -615,34 +616,41 @@ func generateDeltaTgz(goroot string, files []string) (*bytes.Buffer, error) { Mode: 0644, Size: int64(len(version)), }); err != nil { + tw.Close() return nil, err } if _, err := io.WriteString(tw, version); err != nil { + tw.Close() return nil, err } continue } f, err := os.Open(filepath.Join(goroot, file)) if err != nil { + tw.Close() return nil, err } fi, err := f.Stat() if err != nil { f.Close() + tw.Close() return nil, err } header, err := tar.FileInfoHeader(fi, "") if err != nil { f.Close() + tw.Close() return nil, err } header.Name = file // forward slash if err := tw.WriteHeader(header); err != nil { f.Close() + tw.Close() return nil, err } if _, err := io.CopyN(tw, f, header.Size); err != nil { f.Close() + tw.Close() return nil, fmt.Errorf("error copying contents of %s: %v", file, err) } f.Close() diff --git a/cmd/gomote/put.go b/cmd/gomote/put.go index dd272720..850ccd68 100644 --- a/cmd/gomote/put.go +++ b/cmd/gomote/put.go @@ -55,6 +55,7 @@ func legacyPutTar(args []string) error { if err != nil { return err } + defer bc.Close() ctx := context.Background() @@ -210,6 +211,8 @@ func put14(args []string) error { if err != nil { return err } + defer bc.Close() + u := conf.GoBootstrapURL(buildEnv) if u == "" { fmt.Printf("No GoBootstrapURL defined for %q; ignoring. (may be baked into image)\n", name) @@ -264,6 +267,7 @@ func legacyPut(args []string) error { if err != nil { return err } + defer bc.Close() var r io.Reader = os.Stdin var mode os.FileMode = 0666 @@ -385,6 +389,7 @@ func put(args []string) error { func uploadToGCS(ctx context.Context, fields map[string]string, file io.Reader, filename, url string) error { buf := new(bytes.Buffer) mw := multipart.NewWriter(buf) + defer mw.Close() for k, v := range fields { if err := mw.WriteField(k, v); err != nil { diff --git a/cmd/gomote/rdp.go b/cmd/gomote/rdp.go index 84ef85b4..bcc030ff 100644 --- a/cmd/gomote/rdp.go +++ b/cmd/gomote/rdp.go @@ -13,6 +13,7 @@ import ( "log" "net" "os" + "sync" "golang.org/x/build/buildlet" "golang.org/x/sync/errgroup" @@ -41,19 +42,36 @@ func rdp(args []string) error { ln, err := net.Listen("tcp", listen) if err != nil { + bc.Close() return err } + + wg := new(sync.WaitGroup) + defer func() { + // On exiting this function, spin up a goroutine that'll wait + // until all the sync.WaitGroup waits complete, so as to + // properly clean up resources. + go func() { + wg.Wait() + ln.Close() + bc.Close() + }() + }() + log.Printf("Listening on %v to proxy RDP.", ln.Addr()) for { c, err := ln.Accept() if err != nil { return err } - go handleRDPConn(bc, c) + wg.Add(1) + go handleRDPConn(bc, c, wg) } } -func handleRDPConn(bc buildlet.RemoteClient, c net.Conn) { +func handleRDPConn(bc buildlet.RemoteClient, c net.Conn, wg *sync.WaitGroup) { + defer wg.Done() + const Lmsgprefix = 64 // new in Go 1.14, harmless before log := log.New(os.Stderr, c.RemoteAddr().String()+": ", log.LstdFlags|Lmsgprefix) log.Printf("accepted connection, dialing buildlet via coordinator proxy...") diff --git a/cmd/gomote/rm.go b/cmd/gomote/rm.go index 9b427dff..982f174b 100644 --- a/cmd/gomote/rm.go +++ b/cmd/gomote/rm.go @@ -32,6 +32,8 @@ func legacyRm(args []string) error { if err != nil { return err } + defer bc.Close() + ctx := context.Background() return bc.RemoveAll(ctx, args...) } diff --git a/cmd/gomote/run.go b/cmd/gomote/run.go index e3dce9f7..a2b49a75 100644 --- a/cmd/gomote/run.go +++ b/cmd/gomote/run.go @@ -55,6 +55,7 @@ func legacyRun(args []string) error { if err != nil { return err } + defer bc.Close() if builderEnv != "" { altConf, ok := dashboard.Builders[builderEnv] diff --git a/cmd/gomote/ssh.go b/cmd/gomote/ssh.go index 21f9f113..67acfc03 100644 --- a/cmd/gomote/ssh.go +++ b/cmd/gomote/ssh.go @@ -35,10 +35,12 @@ func legacySSH(args []string) error { fs.Usage() } name := fs.Arg(0) - _, err := remoteClient(name) + rc, err := remoteClient(name) if err != nil { return err } + defer rc.Close() + // gomoteUser extracts "gopher" from "user-gopher-linux-amd64-0". gomoteUser := strings.Split(name, "-")[1] githubUser := gophers.GitHubOfGomoteUser(gomoteUser) @@ -133,7 +135,7 @@ func createLocalKeyPair(pubKey, priKey string) error { return cmd.Run() } -func writeCertificateToDisk(b []byte) (string, error) { +func writeCertificateToDisk(b []byte) (_ string, err error) { tmpDir := filepath.Join(os.TempDir(), ".gomote") if err := os.MkdirAll(tmpDir, 0700); err != nil { return "", fmt.Errorf("unable to create temp directory for certficates: %s", err) @@ -142,13 +144,20 @@ func writeCertificateToDisk(b []byte) (string, error) { if err != nil { return "", err } + defer func() { + cerr := tf.Close() + if err == nil { + err = cerr + } + }() + if err := tf.Chmod(0600); err != nil { return "", err } if _, err := tf.Write(b); err != nil { return "", err } - return tf.Name(), tf.Close() + return tf.Name(), nil } func sshConnect(name string, priKey, certPath string) error { diff --git a/cmd/perfrun/perfrun.go b/cmd/perfrun/perfrun.go index ab85d3d3..72333140 100644 --- a/cmd/perfrun/perfrun.go +++ b/cmd/perfrun/perfrun.go @@ -43,6 +43,8 @@ func runBench(out io.Writer, bench, src string, commits []string) error { if err != nil { return err } + defer bc.Close() + log.Printf("Using buildlet %s", bc.RemoteName()) workDir, err := bc.WorkDir(ctx) if err != nil { diff --git a/cmd/release/upload.go b/cmd/release/upload.go index c4e05be6..7c8ca275 100644 --- a/cmd/release/upload.go +++ b/cmd/release/upload.go @@ -187,10 +187,12 @@ func updateSite(f *File) error { func putObject(ctx context.Context, c *storage.Client, name string, body []byte) error { wr := c.Bucket(storageBucket).Object(name).NewWriter(ctx) - if _, err := wr.Write(body); err != nil { - return err + _, err := wr.Write(body) + cerr := wr.Close() + if err == nil { + err = cerr } - return wr.Close() + return err } // expandFiles expands any "/..." paths in GCS URIs to include files in its subtree. diff --git a/internal/coordinator/remote/ssh.go b/internal/coordinator/remote/ssh.go index 9d90318a..82d2bda2 100644 --- a/internal/coordinator/remote/ssh.go +++ b/internal/coordinator/remote/ssh.go @@ -334,6 +334,13 @@ func WriteSSHPrivateKeyToTempFile(key []byte) (path string, err error) { if err != nil { return "", err } + defer func() { + cerr := tf.Close() + if err == nil { + err = cerr + } + }() + if err := tf.Chmod(0600); err != nil { return "", err } diff --git a/internal/gitauth/gitauth.go b/internal/gitauth/gitauth.go index 314cc247..6c98b4f5 100644 --- a/internal/gitauth/gitauth.go +++ b/internal/gitauth/gitauth.go @@ -33,6 +33,8 @@ func Init() error { } sc := secret.MustNewClient() + defer sc.Close() + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() diff --git a/internal/relui/store.go b/internal/relui/store.go index 76ca9467..08b0f1b0 100644 --- a/internal/relui/store.go +++ b/internal/relui/store.go @@ -61,6 +61,8 @@ func MigrateDB(conn string, downUp bool) error { if err != nil { return fmt.Errorf("dbpgx.WithInstance(_, %v) = %v, %w", mcfg, mdb, err) } + defer mdb.Close() + mfs, err := iofs.New(migrations, "migrations") if err != nil { return fmt.Errorf("iofs.New(%v, %q) = %v, %w", migrations, "migrations", mfs, err) diff --git a/internal/task/buildrelease.go b/internal/task/buildrelease.go index 0ac494dd..d97d57a3 100644 --- a/internal/task/buildrelease.go +++ b/internal/task/buildrelease.go @@ -26,7 +26,7 @@ import ( ) // WriteSourceArchive writes a source archive to out, based on revision with version written in as VERSION. -func WriteSourceArchive(ctx *workflow.TaskContext, client *http.Client, gerritURL, revision, version string, out io.Writer) error { +func WriteSourceArchive(ctx *workflow.TaskContext, client *http.Client, gerritURL, revision, version string, out io.Writer) (err error) { ctx.Printf("Create source archive.") tarURL := gerritURL + "/+archive/" + revision + ".tar.gz" resp, err := client.Get(tarURL) @@ -45,6 +45,16 @@ func WriteSourceArchive(ctx *workflow.TaskContext, client *http.Client, gerritUR gzWriter := gzip.NewWriter(out) writer := tar.NewWriter(gzWriter) + defer func() { + cwerr := writer.Close() + cgerr := gzWriter.Close() + if err == nil { + err = cwerr + } + if err == nil { + err = cgerr + } + }() // Add go/VERSION to the archive, and fix up the existing contents. if err := writer.WriteHeader(&tar.Header{ @@ -61,14 +71,11 @@ func WriteSourceArchive(ctx *workflow.TaskContext, client *http.Client, gerritUR if _, err := writer.Write([]byte(version)); err != nil { return err } - if err := adjustTar(reader, writer, "go/", []adjustFunc{ + return adjustTar(reader, writer, "go/", []adjustFunc{ dropRegexpMatches([]string{`VERSION`}), // Don't overwrite our VERSION file from above. dropRegexpMatches(dropPatterns), fixPermissions(), - }); err != nil { - return err - } - + }) if err := writer.Close(); err != nil { return err } @@ -415,7 +422,7 @@ func (b *BuildletStep) runGo(ctx context.Context, args []string, execOpts buildl return b.exec(ctx, goCmd, args, execOpts) } -func ConvertTGZToZIP(r io.Reader, w io.Writer) error { +func ConvertTGZToZIP(r io.Reader, w io.Writer) (err error) { zr, err := gzip.NewReader(r) if err != nil { return err @@ -423,6 +430,12 @@ func ConvertTGZToZIP(r io.Reader, w io.Writer) error { tr := tar.NewReader(zr) zw := zip.NewWriter(w) + defer func() { + cerr := zw.Close() + if err == nil { + err = cerr + } + }() for { th, err := tr.Next() if err == io.EOF { @@ -457,5 +470,5 @@ func ConvertTGZToZIP(r io.Reader, w io.Writer) error { return err } } - return zw.Close() + return } diff --git a/internal/task/tweet.go b/internal/task/tweet.go index 04ce69b2..e14a1972 100644 --- a/internal/task/tweet.go +++ b/internal/task/tweet.go @@ -539,8 +539,10 @@ func (c realTwitterClient) PostTweet(text string, imagePNG []byte) (tweetURL str var buf bytes.Buffer w := multipart.NewWriter(&buf) if f, err := w.CreateFormFile("media", "image.png"); err != nil { + w.Close() return "", err } else if _, err := f.Write(imagePNG); err != nil { + w.Close() return "", err } else if err := w.Close(); err != nil { return "", err diff --git a/maintner/maintnerd/gcslog/gcslog.go b/maintner/maintnerd/gcslog/gcslog.go index 39001194..3cb8d7e3 100644 --- a/maintner/maintnerd/gcslog/gcslog.go +++ b/maintner/maintnerd/gcslog/gcslog.go @@ -447,6 +447,7 @@ func (gl *GCSLog) flushLocked(ctx context.Context) error { w := gl.bucket.Object(objName).NewWriter(ctx) w.ContentType = "application/octet-stream" if _, err := w.Write(buf); err != nil { + w.Close() return err } if err := w.Close(); err != nil { diff --git a/maintner/maintnerd/maintnerd.go b/maintner/maintnerd/maintnerd.go index 6dcfc595..f3fc3b4a 100644 --- a/maintner/maintnerd/maintnerd.go +++ b/maintner/maintnerd/maintnerd.go @@ -314,6 +314,7 @@ func setGodataConfig() { func getGithubToken(ctx context.Context) (string, error) { if metadata.OnGCE() { sc := secret.MustNewClient() + defer sc.Close() ctxSc, cancel := context.WithTimeout(ctx, 10*time.Second) defer cancel() diff --git a/maintner/netsource.go b/maintner/netsource.go index f980a1d2..933f42cc 100644 --- a/maintner/netsource.go +++ b/maintner/netsource.go @@ -654,6 +654,7 @@ func (ns *netMutSource) syncSeg(ctx context.Context, seg LogSegmentJSON) (_ file return fileSeg{}, nil, err } if _, err := tf.Write(newContents); err != nil { + tf.Close() return fileSeg{}, nil, err } if err := tf.Close(); err != nil { diff --git a/maintner/reclog/reclog.go b/maintner/reclog/reclog.go index a1fd4090..af3ab735 100644 --- a/maintner/reclog/reclog.go +++ b/maintner/reclog/reclog.go @@ -116,11 +116,18 @@ func ForeachRecord(r io.Reader, startOffset int64, fn RecordCallback) error { // AppendRecordToFile opens the named filename for append (creating it // if necessary) and adds the provided data record to the end. // The caller is responsible for file locking. -func AppendRecordToFile(filename string, data []byte) error { +func AppendRecordToFile(filename string, data []byte) (err error) { f, err := os.OpenFile(filename, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0600) if err != nil { return err } + defer func() { + cerr := f.Close() + if err == nil { + err = cerr + } + }() + off, err := f.Seek(0, io.SeekEnd) if err != nil { return err @@ -132,11 +139,7 @@ func AppendRecordToFile(filename string, data []byte) error { if off != st.Size() { return fmt.Errorf("Size %v != offset %v", st.Size(), off) } - if err := WriteRecord(f, off, data); err != nil { - f.Close() - return err - } - return f.Close() + return WriteRecord(f, off, data) } // WriteRecord writes the record data to w, formatting the record diff --git a/perfdata/db/db.go b/perfdata/db/db.go index 75ab32cb..b3935f2c 100644 --- a/perfdata/db/db.go +++ b/perfdata/db/db.go @@ -42,11 +42,17 @@ type DB struct { // the same as the parameters for sql.Open. Only mysql and sqlite3 are // explicitly supported; other database engines will receive MySQL // query syntax which may or may not be compatible. -func OpenSQL(driverName, dataSourceName string) (*DB, error) { +func OpenSQL(driverName, dataSourceName string) (_ *DB, rerr error) { db, err := sql.Open(driverName, dataSourceName) if err != nil { return nil, err } + defer func() { + if rerr != nil { + db.Close() + } + }() + if hook := openHooks[driverName]; hook != nil { if err := hook(db); err != nil { return nil, err @@ -229,7 +235,10 @@ func (db *DB) NewUpload(ctx context.Context) (*Upload, error) { } }() var lastID string - err = tx.Stmt(db.lastUpload).QueryRow().Scan(&lastID) + stmt := tx.Stmt(db.lastUpload) + err = stmt.QueryRow().Scan(&lastID) + stmt.Close() + switch err { case sql.ErrNoRows: case nil: @@ -247,7 +256,9 @@ func (db *DB) NewUpload(ctx context.Context) (*Upload, error) { id := fmt.Sprintf("%s.%d", day, num) - _, err = tx.Stmt(db.insertUpload).Exec(id, day, num) + stmt = tx.Stmt(db.insertUpload) + _, err = stmt.Exec(id, day, num) + stmt.Close() if err != nil { return nil, err } diff --git a/tarutil/tarutil.go b/tarutil/tarutil.go index 6dd10b59..4a616f4c 100644 --- a/tarutil/tarutil.go +++ b/tarutil/tarutil.go @@ -67,9 +67,20 @@ func (fl *FileList) TarGz() io.ReadCloser { } } -func (fl *FileList) writeTarGz(w *io.PipeWriter) error { +func (fl *FileList) writeTarGz(w *io.PipeWriter) (err error) { zw := gzip.NewWriter(w) tw := tar.NewWriter(zw) + defer func() { + cterr := tw.Close() + if err == nil { + err = cterr + } + czerr := zw.Close() + if err == nil { + err = czerr + } + }() + for _, f := range fl.files { if err := tw.WriteHeader(f.header); err != nil { return err @@ -81,10 +92,7 @@ func (fl *FileList) writeTarGz(w *io.PipeWriter) error { } } - if err := tw.Close(); err != nil { - return err - } - return zw.Close() + return } // funcCloser implements io.Closer with a function. ```
gopherbot commented 2 years ago

Change https://go.dev/cl/442735 mentions this issue: all: fix resource leaks reported by staticmajor