-
Notifications
You must be signed in to change notification settings - Fork 373
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
flush is not goroutine friendly #113
Comments
You should not call any output functions from multiple goroutines. Only a single goroutine can use output at once. As a bonus you can use input functions from a separate goroutine. But that's all there is. The fact that something is written in Go, doesn't imply that everything is now thread-safe. |
Well, this is definitely one perspective. On the other hand, fmt and log both are concurrent safe. So I'm not sure it's accurate to say that output functions in go aren't by default thread safe. As I said, it's your choice to make -- In my opinion it's a go language wart that there's no easy way to specify what's concurrent friendly and what's not. For your codebase, I'm not sure of the performance impact of wrapping a mutex around that line; it hasn't seemed terrible to me in practice, but I haven't benchmarked it either. On Mon, Dec 28, 2015 at 6:19 PM -0800, "nsf" notifications@github.com wrote: You should not call any output functions from multiple goroutines. Only a single goroutine can use output at once. As a bonus you can use input functions from a separate goroutine. But that's all there is. The fact that something is written in Go, doesn't imply that everything is now thread-safe. — |
Well, this is something I need to document definitely. |
I'm not sure how or if you'd like to address this, but flush gets unhappy if it is being called by multiple goroutines at the same time, and panics. (usually at termbox.go:234).
In my own code, I've wrapped calls to flush with a Mutex, which seems to fix my problems, but note that I haven't run this through the race detector at length to see if things are truly okay with that simple fix.
As a general matter, I think it's nice to maintain concurrency support if possible in a go library, and so I would recommend incorporating something like this, but your call obviously.
The text was updated successfully, but these errors were encountered: