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.

Sign up for GitHub

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

Jump to bottom

cloud.google.com/go/storage: goroutine leak #10948

Closed
peijianju opened this issue Oct 3, 2024 · 6 comments
Closed

cloud.google.com/go/storage: goroutine leak #10948

peijianju opened this issue Oct 3, 2024 · 6 comments
Assignees
@tritone
Labels
api: storage Issues related to the Cloud Storage API. status: investigating The issue is under investigation, which is determined to be non-trivial.

Comments

@peijianju
Copy link

Client

GCP bucket storage

Environment

MacOS local develop environment

Code

cat leak_test.go

package leak

import (
        "testing"

        "go.uber.org/goleak"
	"cloud.google.com/go/storage"
	"context"
	"io"
	"os"
	"time"
	"github.com/stretchr/testify/require"
)

func TestCloudStoageWriter(t *testing.T) {
	defer goleak.VerifyNone(t)

	bucket := "mybucket"
	object := "leakingtest"
	ctx := context.Background()
	client, err := storage.NewClient(ctx)
	require.NoError(t, err)
	defer client.Close()

	// Open local file.
	f, err := os.Open("notes.txt")
	require.NoError(t, err)
	defer f.Close()

	ctx, cancel := context.WithTimeout(ctx, time.Second*50)
	defer cancel()

	o := client.Bucket(bucket).Object(object)

	// Upload an object with storage.Writer.
	wc := o.NewWriter(ctx)
	_, err = io.Copy(wc, f);
	err = wc.Close();
	require.Error(t, err)
}

cat go.mod

module leak

go 1.22.1

require (
	cloud.google.com/go v0.58.0
	cloud.google.com/go/storage v1.8.0
	github.com/stretchr/testify v1.6.1
	go.uber.org/goleak v1.0.0
)

require (
	cloud.google.com/go/bigquery v1.8.0 // indirect
	cloud.google.com/go/datastore v1.1.0 // indirect
	cloud.google.com/go/pubsub v1.3.1 // indirect
	dmitri.shuralyov.com/gpu/mtl v0.0.0-20190408044501-666a987793e9 // indirect
	github.com/BurntSushi/toml v0.3.1 // indirect
	github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802 // indirect
	github.com/census-instrumentation/opencensus-proto v0.2.1 // indirect
	github.com/chzyer/logex v1.1.10 // indirect
	github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect
	github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 // indirect
	github.com/client9/misspell v0.3.4 // indirect
	github.com/cncf/udpa/go v0.0.0-20191209042840-269d4d468f6f // indirect
	github.com/creack/pty v1.1.9 // indirect
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/envoyproxy/go-control-plane v0.9.4 // indirect
	github.com/envoyproxy/protoc-gen-validate v0.1.0 // indirect
	github.com/go-gl/glfw v0.0.0-20190409004039-e6da0acd62b1 // indirect
	github.com/go-gl/glfw/v3.3/glfw v0.0.0-20200222043503-6f7a984d4dc4 // indirect
	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b // indirect
	github.com/golang/groupcache v0.0.0-20200121045136-8c9f03a8e57e // indirect
	github.com/golang/mock v1.4.3 // indirect
	github.com/golang/protobuf v1.4.2 // indirect
	github.com/google/btree v1.0.0 // indirect
	github.com/google/go-cmp v0.4.1 // indirect
	github.com/google/martian v2.1.0+incompatible // indirect
	github.com/google/pprof v0.0.0-20200507031123-427632fa3b1c // indirect
	github.com/google/renameio v0.1.0 // indirect
	github.com/googleapis/gax-go/v2 v2.0.5 // indirect
	github.com/hashicorp/golang-lru v0.5.1 // indirect
	github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6 // indirect
	github.com/jstemmer/go-junit-report v0.9.1 // indirect
	github.com/kisielk/gotool v1.0.0 // indirect
	github.com/kr/pretty v0.1.0 // indirect
	github.com/kr/pty v1.1.1 // indirect
	github.com/kr/text v0.2.0 // indirect
	github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4 // indirect
	github.com/rogpeppe/go-internal v1.3.0 // indirect
	github.com/stretchr/objx v0.1.0 // indirect
	github.com/yuin/goldmark v1.1.27 // indirect
	go.opencensus.io v0.22.3 // indirect
	golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550 // indirect
	golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6 // indirect
	golang.org/x/image v0.0.0-20190802002840-cff245a6509b // indirect
	golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect
	golang.org/x/mobile v0.0.0-20190719004257-d2bd2a29d028 // indirect
	golang.org/x/mod v0.3.0 // indirect
	golang.org/x/net v0.0.0-20200520182314-0ba52f642ac2 // indirect
	golang.org/x/oauth2 v0.0.0-20200107190931-bf48bf16ab8d // indirect
	golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a // indirect
	golang.org/x/sys v0.0.0-20200523222454-059865788121 // indirect
	golang.org/x/text v0.3.2 // indirect
	golang.org/x/time v0.0.0-20191024005414-555d28b269f0 // indirect
	golang.org/x/tools v0.0.0-20200609164405-eb789aa7ce50 // indirect
	golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543 // indirect
	google.golang.org/api v0.26.0 // indirect
	google.golang.org/appengine v1.6.6 // indirect
	google.golang.org/genproto v0.0.0-20200608115520-7c474a2e3482 // indirect
	google.golang.org/grpc v1.29.1 // indirect
	google.golang.org/protobuf v1.24.0 // indirect
	gopkg.in/check.v1 v1.0.0-20200227125254-8fa46927fb4f // indirect
	gopkg.in/errgo.v2 v2.1.0 // indirect
	gopkg.in/yaml.v2 v2.2.2 // indirect
	gopkg.in/yaml.v3 v3.0.0-20200605160147-a5ece683394c // indirect
	honnef.co/go/tools v0.0.1-2020.1.4 // indirect
	rsc.io/binaryregexp v0.2.0 // indirect
	rsc.io/quote/v3 v3.1.0 // indirect
	rsc.io/sampler v1.3.0 // indirect
)

go test ./
--- FAIL: TestCloudStoageWriter (0.91s)
    leaks.go:78: found unexpected goroutines:
        [Goroutine 3 in state select, with go.opencensus.io/stats/view.(*worker).start on top of the stack:
        goroutine 3 [select]:
        go.opencensus.io/stats/view.(*worker).start(0x140001985f0)
        	/Users/peijian/.asdf/installs/golang/1.22.7/packages/pkg/mod/[email protected]/stats/view/worker.go:154 +0x88
        created by go.opencensus.io/stats/view.init.0 in goroutine 1
        	/Users/peijian/.asdf/installs/golang/1.22.7/packages/pkg/mod/[email protected]/stats/view/worker.go:32 +0x88

         Goroutine 53 in state IO wait, with internal/poll.runtime_pollWait on top of the stack:
        goroutine 53 [IO wait]:
        internal/poll.runtime_pollWait(0x103a6b840, 0x72)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/runtime/netpoll.go:345 +0xa0
        internal/poll.(*pollDesc).wait(0x140000ce000?, 0x14000184a00?, 0x0)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/internal/poll/fd_poll_runtime.go:84 +0x28
        internal/poll.(*pollDesc).waitRead(...)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/internal/poll/fd_poll_runtime.go:89
        internal/poll.(*FD).Read(0x140000ce000, {0x14000184a00, 0x1500, 0x1500})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/internal/poll/fd_unix.go:164 +0x200
        net.(*netFD).Read(0x140000ce000, {0x14000184a00?, 0x1037c0068?, 0x1400000eb28?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/fd_posix.go:55 +0x28
        net.(*conn).Read(0x140000761f8, {0x14000184a00?, 0x1400012d8d8?, 0x102b80f4c?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/net.go:179 +0x34
        crypto/tls.(*atLeastReader).Read(0x1400000eb28, {0x14000184a00?, 0x0?, 0x1400000eb28?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:806 +0x40
        bytes.(*Buffer).ReadFrom(0x140003b4d30, {0x1032d4120, 0x1400000eb28})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/bytes/buffer.go:211 +0x90
        crypto/tls.(*Conn).readFromUntil(0x140003b4a88, {0x14a641430, 0x140000761f8}, 0x1400012d9b0?)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:828 +0xd0
        crypto/tls.(*Conn).readRecordOrCCS(0x140003b4a88, 0x0)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:626 +0x35c
        crypto/tls.(*Conn).readRecord(...)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:588
        crypto/tls.(*Conn).Read(0x140003b4a88, {0x14000138000, 0x1000, 0x1032dbe38?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:1370 +0x168
        bufio.(*Reader).Read(0x14000136c00, {0x14000134580, 0x9, 0x140003b4a88?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/bufio/bufio.go:241 +0x1b4
        io.ReadAtLeast({0x1032d30f8, 0x14000136c00}, {0x14000134580, 0x9, 0x9}, 0x9)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/io/io.go:335 +0xa0
        io.ReadFull(...)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/io/io.go:354
        net/http.http2readFrameHeader({0x14000134580, 0x9, 0x14001504a40?}, {0x1032d30f8?, 0x14000136c00?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:1638 +0x58
        net/http.(*http2Framer).ReadFrame(0x14000134540)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:1902 +0x78
        net/http.(*http2clientConnReadLoop).run(0x1400012df98)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:9303 +0xf8
        net/http.(*http2ClientConn).readLoop(0x1400033c300)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:9198 +0x5c
        created by net/http.(*http2Transport).newClientConn in goroutine 52
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:7848 +0xa84

         Goroutine 33 in state IO wait, with internal/poll.runtime_pollWait on top of the stack:
        goroutine 33 [IO wait]:
        internal/poll.runtime_pollWait(0x103a6b748, 0x72)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/runtime/netpoll.go:345 +0xa0
        internal/poll.(*pollDesc).wait(0x140000ce200?, 0x140001cc000?, 0x0)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/internal/poll/fd_poll_runtime.go:84 +0x28
        internal/poll.(*pollDesc).waitRead(...)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/internal/poll/fd_poll_runtime.go:89
        internal/poll.(*FD).Read(0x140000ce200, {0x140001cc000, 0x1300, 0x1300})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/internal/poll/fd_unix.go:164 +0x200
        net.(*netFD).Read(0x140000ce200, {0x140001cc000?, 0x14a5b8be8?, 0x1400150a498?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/fd_posix.go:55 +0x28
        net.(*conn).Read(0x14001508080, {0x140001cc000?, 0x140002068d8?, 0x102b80f4c?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/net.go:179 +0x34
        crypto/tls.(*atLeastReader).Read(0x1400150a498, {0x140001cc000?, 0x0?, 0x102e07af8?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:806 +0x40
        bytes.(*Buffer).ReadFrom(0x140001882b0, {0x1032d4120, 0x1400150a498})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/bytes/buffer.go:211 +0x90
        crypto/tls.(*Conn).readFromUntil(0x14000188008, {0x14a641430, 0x14001508080}, 0x140002069b0?)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:828 +0xd0
        crypto/tls.(*Conn).readRecordOrCCS(0x14000188008, 0x0)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:626 +0x35c
        crypto/tls.(*Conn).readRecord(...)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:588
        crypto/tls.(*Conn).Read(0x14000188008, {0x140001e8000, 0x1000, 0x1032dbe38?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/crypto/tls/conn.go:1370 +0x168
        bufio.(*Reader).Read(0x140000fb3e0, {0x140001e23c0, 0x9, 0x14000188008?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/bufio/bufio.go:241 +0x1b4
        io.ReadAtLeast({0x1032d30f8, 0x140000fb3e0}, {0x140001e23c0, 0x9, 0x9}, 0x9)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/io/io.go:335 +0xa0
        io.ReadFull(...)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/io/io.go:354
        net/http.http2readFrameHeader({0x140001e23c0, 0x9, 0x140003d28c0?}, {0x1032d30f8?, 0x140000fb3e0?})
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:1638 +0x58
        net/http.(*http2Framer).ReadFrame(0x140001e2380)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:1902 +0x78
        net/http.(*http2clientConnReadLoop).run(0x14000206f98)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:9303 +0xf8
        net/http.(*http2ClientConn).readLoop(0x140001e6000)
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:9198 +0x5c
        created by net/http.(*http2Transport).newClientConn in goroutine 32
        	/Users/peijian/.asdf/installs/golang/1.22.1/go/src/net/http/h2_bundle.go:7848 +0xa84
        ]
FAIL
FAIL	leak	1.587s
FAIL
@peijianju peijianju added the triage me I really want to be triaged. label Oct 3, 2024
@codyoss codyoss added the api: storage Issues related to the Cloud Storage API. label Oct 3, 2024
@tritone tritone added status: investigating The issue is under investigation, which is determined to be non-trivial. and removed triage me I really want to be triaged. labels Oct 3, 2024
@tritone
Copy link
Contributor

tritone commented Oct 3, 2024

@peijianju thanks for reporting. A few questions:

  1. What version of the storage library did you use? Is this a regression that arises only in new versions?
  2. Did you see this come up in practice as a memory leak running in production systems?

@tritone
Copy link
Contributor

tritone commented Oct 3, 2024

Actually, looks like from your go.mod that you are on a very old version of storage. Could you try the latest release (storage/v1.43.0)?

@peijianju
Copy link
Author

Thank you @tritone , update my go.mod to use cloud.google.com/go/storage v1.44.0. Still suffer from leaking.

What version of the storage library did you use? Is this a regression that arises only in new versions?

New version v1.44.0 is used

Did you see this come up in practice as a memory leak running in production systems?

No, this is found during my development, our own test suit keep reporting leaking.
So I tried to use the official example at https://cloud.google.com/storage/docs/uploading-objects#storage-upload-object-go. Same leaking.

No report from production systems yet.

module golang-quicky

go 1.22.6

require (
	cloud.google.com/go/storage v1.44.0
	github.com/stretchr/testify v1.9.0
	go.uber.org/goleak v1.3.0
)

require (
	cel.dev/expr v0.16.1 // indirect
	cloud.google.com/go v0.115.1 // indirect
	cloud.google.com/go/auth v0.9.3 // indirect
	cloud.google.com/go/auth/oauth2adapt v0.2.4 // indirect
	cloud.google.com/go/compute/metadata v0.5.1 // indirect
	cloud.google.com/go/iam v1.2.1 // indirect
	cloud.google.com/go/monitoring v1.21.0 // indirect
	github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp v1.24.1 // indirect
	github.com/GoogleCloudPlatform/opentelemetry-operations-go/exporter/metric v0.48.1 // indirect
	github.com/GoogleCloudPlatform/opentelemetry-operations-go/internal/resourcemapping v0.48.1 // indirect
	github.com/census-instrumentation/opencensus-proto v0.4.1 // indirect
	github.com/cespare/xxhash/v2 v2.3.0 // indirect
	github.com/cncf/xds/go v0.0.0-20240905190251-b4127c9b8d78 // indirect
	github.com/davecgh/go-spew v1.1.1 // indirect
	github.com/envoyproxy/go-control-plane v0.13.0 // indirect
	github.com/envoyproxy/protoc-gen-validate v1.1.0 // indirect
	github.com/felixge/httpsnoop v1.0.4 // indirect
	github.com/go-logr/logr v1.4.2 // indirect
	github.com/go-logr/stdr v1.2.2 // indirect
	github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect
	github.com/golang/protobuf v1.5.4 // indirect
	github.com/google/s2a-go v0.1.8 // indirect
	github.com/google/uuid v1.6.0 // indirect
	github.com/googleapis/enterprise-certificate-proxy v0.3.4 // indirect
	github.com/googleapis/gax-go/v2 v2.13.0 // indirect
	github.com/kr/text v0.2.0 // indirect
	github.com/planetscale/vtprotobuf v0.6.1-0.20240319094008-0393e58bdf10 // indirect
	github.com/pmezard/go-difflib v1.0.0 // indirect
	go.opencensus.io v0.24.0 // indirect
	go.opentelemetry.io/contrib/detectors/gcp v1.29.0 // indirect
	go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.54.0 // indirect
	go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.54.0 // indirect
	go.opentelemetry.io/otel v1.29.0 // indirect
	go.opentelemetry.io/otel/metric v1.29.0 // indirect
	go.opentelemetry.io/otel/sdk v1.29.0 // indirect
	go.opentelemetry.io/otel/sdk/metric v1.29.0 // indirect
	go.opentelemetry.io/otel/trace v1.29.0 // indirect
	golang.org/x/crypto v0.27.0 // indirect
	golang.org/x/net v0.29.0 // indirect
	golang.org/x/oauth2 v0.23.0 // indirect
	golang.org/x/sync v0.8.0 // indirect
	golang.org/x/sys v0.25.0 // indirect
	golang.org/x/text v0.18.0 // indirect
	golang.org/x/time v0.6.0 // indirect
	google.golang.org/api v0.197.0 // indirect
	google.golang.org/genproto v0.0.0-20240903143218-8af14fe29dc1 // indirect
	google.golang.org/genproto/googleapis/api v0.0.0-20240903143218-8af14fe29dc1 // indirect
	google.golang.org/genproto/googleapis/rpc v0.0.0-20240903143218-8af14fe29dc1 // indirect
	google.golang.org/grpc v1.66.2 // indirect
	google.golang.org/grpc/stats/opentelemetry v0.0.0-20240907200651-3ffb98b2c93a // indirect
	google.golang.org/protobuf v1.34.2 // indirect
	gopkg.in/yaml.v3 v3.0.1 // indirect
)

@stanhu
Copy link
Contributor

stanhu commented Oct 7, 2024

I'm pretty sure these are false positives. The OpenCensus Goroutine is a known issue (census-instrumentation/opencensus-go#1191) since the init() function creates this at startup. The HTTP2 Goroutines stick around because they are idle, cached connections designed to speed things up: golang/go#25759 (comment)

We've been using GoCloud with this module to upload files to Google Cloud Storage and have not seen memory leaks in production. We actively monitor memory usage, and you can see in June 2024 when we discovered a memory leak due to another unrelated issue after the upgrade to Go 1.22:

image

@peijianju
Copy link
Author

Thank you @stanhu , I am closing this as a false alarm.

I am looking at how to ignore these. For, census-instrumentation/opencensus-go#1191, it seems straightforward by goleak.IgnoreTopFunction("go.opencensus.io/stats/view.(*worker).start"),

But for golang/go#25759 (comment), some suggested to use CloseIdleConnections(), but this function is from http.client, not sure how storage client can use that.
Tried some suggestions in golang/go#25759 (comment), e.g. defer http.DefaultTransport.(*http.Transport).CloseIdleConnections() or use oauth2.HTTPClient to retrieve http.client. No luck though.

@tritone
Copy link
Contributor

tritone commented Oct 9, 2024

I think re: CloseIdleConnections(), Client.Close() could call this internally. On the other hand 1. the HTTP client used by storage.Client could be shared with other users and 2. Looks like the idle connections will get garbage collected in a reasonable amount of time regardless. So, I think in practice this should not be a problem unless the user is e.g. creating a new client for each request (which we specifically warn against).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tritone tritone

Labels
api: storage Issues related to the Cloud Storage API. status: investigating The issue is under investigation, which is determined to be non-trivial.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@stanhu @tritone @codyoss @peijianju