Ok, I believe this resolves the root issue(s) uncovered by @JeeChoi in #11, namely:
subscription having it's own context and cancel routine was definitely odd
the channel returned by subscription.Ch() was never closed
However, when adding code to close the channel, I realized there was a more fundamental issue, in that the channel was being written to by multiple go routines, so closing the channel could lead to race conditions where writes occurred after close, leading to panics. After researching solutions, the one I ended up liking best was a modified version of the code in "variant 5" one this page: https://go101.org/article/channel-closing.html
I added a fair amount of comments since the number of channels ballooned from one to four, and also wrapped all the logic in private methods and a constructor for subscription instead of sprinkling it through loop.go, so overall I hope the code is easier to read. Finally, I added some rather contrived code to the example to show how unsubscribing can be used.
Ideally we'd have better test coverage for loop.go, but that would require either making httptest support websockets or writing a pretty extensive IPC-based mock, either one is something to consider for later but not something I think we have the bandwidth to tackle now.
Ok, I believe this resolves the root issue(s) uncovered by @JeeChoi in #11, namely:
subscription.Ch()was never closedHowever, when adding code to close the channel, I realized there was a more fundamental issue, in that the channel was being written to by multiple go routines, so closing the channel could lead to race conditions where writes occurred after close, leading to panics. After researching solutions, the one I ended up liking best was a modified version of the code in "variant 5" one this page: https://go101.org/article/channel-closing.html
I added a fair amount of comments since the number of channels ballooned from one to four, and also wrapped all the logic in private methods and a constructor for subscription instead of sprinkling it through loop.go, so overall I hope the code is easier to read. Finally, I added some rather contrived code to the example to show how unsubscribing can be used.
Ideally we'd have better test coverage for loop.go, but that would require either making httptest support websockets or writing a pretty extensive IPC-based mock, either one is something to consider for later but not something I think we have the bandwidth to tackle now.