turnpike icon indicating copy to clipboard operation
turnpike copied to clipboard

client: crash in client.waitOnListener

Open avdva opened this issue 7 years ago • 1 comments

panic: send on closed channel
goroutine 149 [running]:
panic(0xa69500, 0xc82230dee0)
        /usr/lib/go/src/runtime/panic.go:464 +0x3e6
olymptrade.com/feed-poloniex/vendor/github.com/jcelliott/turnpike.(*Client).waitOnListener(0xc82311bb30, 0x5f8817d54f0f, 0x0, 0x0, 0x7fda6b395028, 0xc82230ded0)
        /var/lib/jenkins/workspace/feed-poloniex-adapter-build/src/olymptrade.com/feed-poloniex/vendor/github.com/jcelliott/turnpike/client.go:385 +0x56b
olymptrade.com/feed-poloniex/vendor/github.com/jcelliott/turnpike.(*Client).Subscribe(0xc82311bb30, 0xc820340b10, 0x8, 0xc822fd0390, 0xc8226d4a30, 0x0, 0x0)
        /var/lib/jenkins/workspace/feed-poloniex-adapter-build/src/olymptrade.com/feed-poloniex/vendor/github.com/jcelliott/turnpike/client.go:410 +0x1c7
...

Its not safe to close acts channel, some goroutines may send there causing such crashes. Here I rewrote this logic, and it seems to work fine. Also, I added a feature of parsing json numbers as json.Number, #129. Take a look at it please, and if everything is ok, I'll do a PR.

avdva avatar Nov 20 '17 09:11 avdva

This looks nice and I would like if this was merged. I'm no contributor of this however...

One thing though, NewWebsocketPeer(MSGPACK, fmt.Sprintf("ws://localhost:%d/", port), nil, nil, nil) All these nil's starts to get a bit weird, wouldn't it be better if NewWebsocketPeer takes a "options" struct or something instead?

What do you think @mourad and @avdva?

wstrm avatar Nov 22 '17 09:11 wstrm