-
Notifications
You must be signed in to change notification settings - Fork 191
Add Close() method to Project to release resources #451
base: master
Are you sure you want to change the base?
Conversation
0ef587a
to
e07b966
Compare
e07b966
to
eb0d286
Compare
1889c7c
to
57ec43a
Compare
There is another goroutine leak in the |
Right |
It's a bit tricky to fix the leak by the default client factory. Because the factory returns the same instance of Also, Here is a workaround with a custom client factory that implements |
@yudai ping as it seems there is a build failure 😭 |
57ec43a
to
a9fa9ac
Compare
Problem Observed Goroutine leaks because there is no way to "close" projects Problem Detail NewProject() creates a defaultListener in it. The listener starts a new goroutine in NewDefaultListener(). We currently don't have a method to stop this goroutine. Suggested Resolution This commit adds a new method Close() to release resources tied to a Project. Signed-off-by: Iwasaki Yudai <[email protected]>
a9fa9ac
to
6cce1d7
Compare
@vdemeester fixed it :) |
Hi, thank you for the great library.
I found a goroutine leak in the lib, so created a this PR.
It introduces a new method to close a channel, which means it requires a minor API change.
Problem Observed
Goroutine leaks because there is no way to "close" projects.
Problem Detail
NewProject()
creates adefaultListener
in it.The listener starts a new goroutine in
NewDefaultListener()
.We currently don't have a method to stop this goroutine.
Suggested Resolution
This commit adds a new method
Close()
to release resourcestied to a
Project
.