Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
odeke-em opened this issue Oct 13, 2022 · 1 comment
Open

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

odeke-em opened this issue Oct 13, 2022 · 1 comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@odeke-em
Copy link
Member

odeke-em commented Oct 13, 2022

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:

  • there are sql.Statements that are unclosed because they were created like this _, err = tx.Stmt(db.insertUpload).Exec(id, day, num)
  • unclosed multipart.Writers
  • numerous unclosed writers

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.

</details>

@gopherbot gopherbot added the Builders x/build issues (builders, bots, dashboards) label Oct 13, 2022
@gopherbot gopherbot added this to the Unreleased milestone Oct 13, 2022
@gopherbot
Copy link

gopherbot commented Oct 13, 2022

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

@joedian joedian added the NeedsFix The path to resolution is known, but the work has not been done. label Oct 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

3 participants