Possible goroutine leak

[erikstmartin]

After a watch has been seen it's send down the Event chan, and then deleted from the watchers list here https://github.com/samuel/go-zookeeper/blob/master/zk/conn.go#L496 then when watchers are invalidated, the map is recreated here https://github.com/samuel/go-zookeeper/blob/master/zk/conn.go#L258

In both these cases the Event channels are basically lost, all references are dropped from go-zookeeper, but ultimately this could lead to leaking of goroutines on the calling side, or indefinite blocking as there is never an opportunity for a receive on this channel.

Ideally we'd want to close the channel before we loose our references to the channel so that we essentially notify the caller there will be no subsequent sends on the channel, and can be trapped via ev, _ok := <- eventChan so that the caller can continue on.

Thoughts?

[samuel]

In both cases an event is sent down the channel before dropping the references. The event channels are single shot.. that is, you'll only ever receive a single event on them. Zookeeper works by requiring the client to recreate watches after they're signalled. Might make sense to close the channel anyway though.. to avoid deadlocks on buggy code trying to receive more than one event on the channel.

[erikstmartin]

Right, my title probably could have been more descriptive. There isn't a leak within go-zookeeper itself, it just helps facilitate the caller making bad choices if they aren't paying attention.

[samuel]

Ahh, got it. Thanks. I'll make it close the channels.