8000 make go routines shutdown gracefully by geyslan · Pull Request #2784 · aquasecurity/tracee · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

make go routines shutdown gracefully #2784

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
Merged
merged 1 commit into from
Mar 16, 2023
Merged

make go routines shutdown gracefully #2784

merged 1 commit into from
Mar 16, 2023

Conversation

geyslan
Copy link
Member
@geyslan geyslan commented Mar 1, 2023 •
edited
Loading

1. Explain what the PR does

fix go routines leaks

Fixes: #2763

2. Explain how to test it

One can detect the go routine leakage running sudo make test-integration with the help of goleak.

        "github.com/stretchr/testify/require"
+       "go.uber.org/goleak"
 )
...
 func Test_EventFilters(t *testing.T) {
+       defer goleak.VerifyNone(t)
+
        testCases := []struct {
                name       string

3. Other comments

@geyslan geyslan self-assigned this Mar 1, 2023
@geyslan geyslan force-pushed the 2763-fix-go-leaks branch from c888206 to 1ea0283 Compare March 9, 2023 21:29
@geyslan
Copy link
Member Author
geyslan commented Mar 9, 2023

There's still one last leak, this time in the libbpfgo. @rafaeldtinoco WDYT?

   leaks.go:78: found unexpected goroutines:
        [Goroutine 421 in state syscall, with github.com/aquasecurity/libbpfgo._Cfunc_bpf_link__destroy on top of the stack:
        goroutine 421 [syscall]:
        github.com/aquasecurity/libbpfgo._Cfunc_bpf_link__destroy(0x7f942c710290)
                _cgo_gotypes.go:554 +0x4c
        github.com/aquasecurity/libbpfgo.(*BPFLink).Destroy.func1(0xa9ee1c?)
                /root/go/pkg/mod/github.com/aquasecurity/libbpfgo@v0.4.6-libbpf-1.1.0/libbpfgo.go:195 +0x46
        github.com/aquasecurity/libbpfgo.(*BPFLink).Destroy(0xc000fa98c0)
                /root/go/pkg/mod/github.com/aquasecurity/libbpfgo@v0.4.6-libbpf-1.1.0/libbpfgo.go:195 +0x25
        github.com/aquasecurity/tracee/pkg/ebpf/probes.(*traceProbe).detach(0xc0016e0570, {0xc003749b38?, 0xac5886?, 0xc0009396e0?})
                /home/gg/code/tracee/pkg/ebpf/probes/trace.go:82 +0x36
        github.com/aquasecurity/tracee/pkg/ebpf/probes.(*probes).DetachAll(0xc0009396f8?)
                /home/gg/code/tracee/pkg/ebpf/probes/probes.go:188 +0x92
        github.com/aquasecurity/tracee/pkg/ebpf.(*Tracee).Close.func1()
                /home/gg/code/tracee/pkg/ebpf/tracee.go:1428 +0x3a
        github.com/aquasecurity/tracee/pkg/capabilities.(*Capabilities).Required(0xc0002930c0, 0xc003749c68?)
                /home/gg/code/tracee/pkg/capabilities/capabilities.go:174 +0xc4
        github.com/aquasecurity/tracee/pkg/ebpf.(*Tracee).Close(0xc000612500)
                /home/gg/code/tracee/pkg/ebpf/tracee.go:1425 +0x89
        github.com/aquasecurity/tracee/pkg/ebpf.(*Tracee).Run(0xc000612500, {0x22a9d20, 0xc0010a0000})
                /home/gg/code/tracee/pkg/ebpf/tracee.go:1419 +0x8a5
        created by github.com/aquasecurity/tracee/tests/integration.startTracee
                /home/gg/code/tracee/tests/integration/tracee.go:72 +0x3f1
        ]
--- FAIL: Test_EventFilters (29.57s)

@geyslan geyslan marked this pull request as ready for review March 9, 2023 21:44
NDStrahilevitz
NDStrahilevitz approved these changes Mar 13, 2023
View reviewed changes
Copy link
Collaborator
@NDStrahilevitz NDStrahilevitz left a comment

Choose a reason for hiding this comment

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

LGTM, your call if you want to merge before fixing the libbpfgo one or commit the bump for that on top.

@geyslan geyslan force-pushed the 2763-fix-go-leaks branch from 1ea0283 to 13c48ff Compare March 13, 2023 17:46
@geyslan
Copy link
Member Author
geyslan commented Mar 13, 2023

LGTM, your call if you want to merge before fixing the libbpfgo one or commit the bump for that on top.

Let's merge it. I'll raise a libbpfgo issue to be tackled in the future.

@geyslan geyslan mentioned this pull request Mar 13, 2023
Closed
@geyslan geyslan requested a review from NDStrahilevitz March 13, 2023 17:57
@rafaeldtinoco
Copy link
Contributor

Please rebase to HEAD before trying tests again. The rate limit issue with docker hub has been fixed.

@geyslan geyslan force-pushed the 2763-fix-go-leaks branch from 13c48ff to 26b9f8c Compare March 13, 2023 22:39
@geyslan
Copy link
Member Author
geyslan commented Mar 13, 2023

Please rebase to HEAD before trying tests again. The rate limit issue with docker hub has been fixed.

After rebase it.

image

@rafaeldtinoco
Copy link
Contributor

After rebase it.

Working on it.

@rafaeldtinoco
Copy link
Contributor

Sorry for the noise, we were reaching docker hub rate limits and I tried something that did not work. Could you please rebase and try the tests again ? Thanks!

@geyslan geyslan force-pushed the 2763-fix-go-leaks branch from 26b9f8c to 4713b24 Compare March 14, 2023 13:28
@geyslan geyslan force-pushed the 2763-fix-go-leaks branch from 4713b24 to 85d9ffe Compare March 16, 2023 14:37
@geyslan geyslan merged commit 1b5abef into aquasecurity:main Mar 16, 2023
@geyslan geyslan deleted the 2763-fix-go-leaks branch May 29, 2023 22:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NDStrahilevitz NDStrahilevitz NDStrahilevitz approved these changes

Assignees

@geyslan geyslan

Labels
area/ebpf area/testing area/UX kind/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integration Tests go routine leakage
3 participants
@geyslan @rafaeldtinoco @NDStrahilevitz
0