Skip to content

client/pkg/testutil: update interestingGoroutines#18287

Merged
serathius merged 1 commit intoetcd-io:mainfrom
fuweid:update-goleak
Jul 6, 2024
Merged

client/pkg/testutil: update interestingGoroutines#18287
serathius merged 1 commit intoetcd-io:mainfrom
fuweid:update-goleak

Conversation

@fuweid
Copy link
Member

@fuweid fuweid commented Jul 5, 2024

The Go runtime uses runtime Finalizer to delete cert [1]. The interestingGoroutines is able to collect stack like,

leak.go:103: Found leaked goroutined BEFORE test appears to have leaked :
        sync.(*Map).LoadAndDelete(0xc00031e180, {0xe07320, 0xc00009fde0})
                /usr/local/go/src/sync/map.go:272 +0x192
        sync.(*Map).Delete(...)
                /usr/local/go/src/sync/map.go:297
        crypto/tls.(*certCache).evict(...)
                /usr/local/go/src/crypto/tls/cache.go:73
        crypto/tls.(*certCache).active.func1(0x0?)
                /usr/local/go/src/crypto/tls/cache.go:65 +0x67

It's caused by GC instead of leaky goroutine. interestingGoroutines should skip it.

Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.


REF: #18274 (comment)

The Go runtime uses runtime Finalizer to delete cert [[1]]. The
interestingGoroutines is able to collect stack like,

```plain
leak.go:103: Found leaked goroutined BEFORE test appears to have leaked :
        sync.(*Map).LoadAndDelete(0xc00031e180, {0xe07320, 0xc00009fde0})
                /usr/local/go/src/sync/map.go:272 +0x192
        sync.(*Map).Delete(...)
                /usr/local/go/src/sync/map.go:297
        crypto/tls.(*certCache).evict(...)
                /usr/local/go/src/crypto/tls/cache.go:73
        crypto/tls.(*certCache).active.func1(0x0?)
                /usr/local/go/src/crypto/tls/cache.go:65 +0x67
```

It's caused by GC instead of leaky goroutine. interestingGoroutines
should skip it.

[1]: https://github.com/golang/go/blob/8e1fdea8316d840fd07e9d6e026048e53290948b/src/crypto/tls/cache.go#L63

Signed-off-by: Wei Fu <fuweid89@gmail.com>
Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@k8s-ci-robot
Copy link

@fuweid: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
pull-etcd-robustness-amd64 ccaea7e link false /test pull-etcd-robustness-amd64

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

@fuweid
Copy link
Member Author

fuweid commented Jul 6, 2024

/retest

@k8s-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ahrtr, fuweid, serathius

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@serathius serathius merged commit 24e0599 into etcd-io:main Jul 6, 2024
@fuweid fuweid deleted the update-goleak branch August 17, 2024 04:33
fuweid added a commit to fuweid/etcd that referenced this pull request Aug 17, 2024
The Go runtime uses runtime Finalizer to delete cert [[1]]. The
interestingGoroutines is able to collect stack like,

```plain
leak.go:103: Found leaked goroutined BEFORE test appears to have leaked :
        sync.(*Map).LoadAndDelete(0xc00031e180, {0xe07320, 0xc00009fde0})
                /usr/local/go/src/sync/map.go:272 +0x192
        sync.(*Map).Delete(...)
                /usr/local/go/src/sync/map.go:297
        crypto/tls.(*certCache).evict(...)
                /usr/local/go/src/crypto/tls/cache.go:73
        crypto/tls.(*certCache).active.func1(0x0?)
                /usr/local/go/src/crypto/tls/cache.go:65 +0x67
```

It's caused by GC instead of leaky goroutine. interestingGoroutines
should skip it.

Backport of etcd-io#18287

[1]: https://github.com/golang/go/blob/8e1fdea8316d840fd07e9d6e026048e53290948b/src/crypto/tls/cache.go#L63

Signed-off-by: Wei Fu <fuweid89@gmail.com>
fuweid added a commit to fuweid/etcd that referenced this pull request Aug 17, 2024
The Go runtime uses runtime Finalizer to delete cert [[1]]. The
interestingGoroutines is able to collect stack like,

```plain
leak.go:103: Found leaked goroutined BEFORE test appears to have leaked :
        sync.(*Map).LoadAndDelete(0xc00031e180, {0xe07320, 0xc00009fde0})
                /usr/local/go/src/sync/map.go:272 +0x192
        sync.(*Map).Delete(...)
                /usr/local/go/src/sync/map.go:297
        crypto/tls.(*certCache).evict(...)
                /usr/local/go/src/crypto/tls/cache.go:73
        crypto/tls.(*certCache).active.func1(0x0?)
                /usr/local/go/src/crypto/tls/cache.go:65 +0x67
```

It's caused by GC instead of leaky goroutine. interestingGoroutines
should skip it.

Backport of etcd-io#18287

[1]: https://github.com/golang/go/blob/8e1fdea8316d840fd07e9d6e026048e53290948b/src/crypto/tls/cache.go#L63

Signed-off-by: Wei Fu <fuweid89@gmail.com>
aneesh1 pushed a commit to DataDog/etcd that referenced this pull request Sep 25, 2024
The Go runtime uses runtime Finalizer to delete cert [[1]]. The
interestingGoroutines is able to collect stack like,

```plain
leak.go:103: Found leaked goroutined BEFORE test appears to have leaked :
        sync.(*Map).LoadAndDelete(0xc00031e180, {0xe07320, 0xc00009fde0})
                /usr/local/go/src/sync/map.go:272 +0x192
        sync.(*Map).Delete(...)
                /usr/local/go/src/sync/map.go:297
        crypto/tls.(*certCache).evict(...)
                /usr/local/go/src/crypto/tls/cache.go:73
        crypto/tls.(*certCache).active.func1(0x0?)
                /usr/local/go/src/crypto/tls/cache.go:65 +0x67
```

It's caused by GC instead of leaky goroutine. interestingGoroutines
should skip it.

Backport of etcd-io#18287

[1]: https://github.com/golang/go/blob/8e1fdea8316d840fd07e9d6e026048e53290948b/src/crypto/tls/cache.go#L63

Signed-off-by: Wei Fu <fuweid89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

4 participants