Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

client: crash in client.waitOnListener #142

Open
avdva opened this issue Nov 20, 2017 · 1 comment
Open

client: crash in client.waitOnListener #142

avdva opened this issue Nov 20, 2017 · 1 comment

Comments

@avdva
Copy link

avdva commented Nov 20, 2017

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.

@wstrm
Copy link

wstrm commented Nov 22, 2017

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants