Adds Signal.WaitForStateMinWithCleanup to avoid leaking waiters
#397
Conversation
This converts the `Signal.waiters` list into a map and adds a `WaitForStateMinWithCleanup` that returns a function to allow the caller to remove the waiter from the list when it is done.
Adds Signal.WaitForStateMinWithCleanup
| @@ -107,6 +109,26 @@ func (s *Signal) WaitForStateMin(state State) <-chan struct{} { | |||
| return s.waitForWaiter(state, w) | |||
| } | |||
|
|
|||
| // WaitForStateMinWithCleanup functions identically to WaitForStateMin, but returns a cleanup function in addition | |||
| // so that the caller can cleanup resources if it no longer wants to wait | |||
| func (s *Signal) WaitForStateMinWithCleanup(state State) (<-chan struct{}, func()) { | |||
There was a problem hiding this comment.
The name sounds a bit clumsy to be honest, almost like java :D, but I don't have a better idea and introducing breaking changes just for this seems overkill. The signal implementation never was the finest piece of software in the first place, so maybe let's just do it the way you proposed.
There was a problem hiding this comment.
Yeah, I struggled a bit with the naming 😆
If there are more cases where this cleanup is needed down the road, the interface could be simplified and just always provide it.
There was a problem hiding this comment.
@frairon any idea what's up with the actions failures? It seems to be because it's running from my fork?
There was a problem hiding this comment.
very sorry for the long delay. Let's just keep it that way and merge it.
Also, don't worry about the actions, they're quite flaky right now and we'll try to take care of it.
|
sorry again for the very long delay, I'll merge that now, many thanks for the fix! |
If
Signal.WaitForState*is called multiple times for a state like "stopping", the waiters will remain in the signal's queue forever and their channel will remain open. This has happened in my case while callingPartitionProcessor.VisitValuesmultiple times. This was causingSignal.waitersto grow indefinitely and cause a memory leak.This change adds a cleanup function to allow a caller to remove itself from the
Signal.waiterslist if it no longer cares about waiting for the given state.fixes #398