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

iterator method of treeSet has goroutine leaks #52

Closed
zonewave opened this issue Apr 30, 2023 · 1 comment · Fixed by #53
Closed

iterator method of treeSet has goroutine leaks #52

zonewave opened this issue Apr 30, 2023 · 1 comment · Fixed by #53

Comments

@zonewave
Copy link
Contributor

I use "goleak" to detector leak:

	t.Run("diff set", func(t *testing.T) {
		defer goleak.VerifyNone(t)
		t1 := TreeSetFrom[int, Compare[int]]([]int{1, 2, 3}, Cmp[int])
		t2 := TreeSetFrom[int, Compare[int]]([]int{4, 5, 6}, Cmp[int])
		must.False(t, t1.Subset(t2))
	})

if not iter all node, the channel will block
image

zonewave pushed a commit to zonewave/go-set that referenced this issue Apr 30, 2023
1. use goleak
2. infix: add bool to return param of visit
3, iterate: add context to control
zonewave pushed a commit to zonewave/go-set that referenced this issue Apr 30, 2023
1. use goleak
2. infix: add bool to return param of visit
3, iterate: add context to control
zonewave pushed a commit to zonewave/go-set that referenced this issue May 1, 2023
1. use goleak
2. infix: add bool to return param of visit
3, iterate: add context to control
@shoenig
Copy link
Member

shoenig commented May 1, 2023

Good catch, I had been thinking about ways to iterate 2 trees (for the .Equal and .Subset cases) without the use of goroutines, but after so many years of programming in Go using goroutines is just easier.

If we could eliminate the use of goroutines though, that would be even better!

shoenig pushed a commit that referenced this issue May 1, 2023
1. use goleak
2. infix: add bool to return param of visit
3, iterate: add context to control
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants