On Wed, Mar 13, 2019 at 4:01 PM Andrey Tcherepanov < xnow4fippy...@sneakemail.com> wrote:
> Roger, it is not that I wondering it is async assignments are bad, it is > more of a question "why does it have to be SO complicated"? > > (please forgive me for a long ranty post) > > There were couple of things I was implementing for our little in-house > server app. > One was (in)famous fan-out pattern for broadcasting messages to clients, > and one for job queue(s) that could run fast (seconds) or long (hours, > days) where items kind of coming it from multiple (same) clients. > > While implementing notifications part, I have eaten my learning salty cake > of naivety with lockings and crashes on top. So second (jobs control) was > not using channels in any form at all. (I guess I also got influenced by > looking into Go runtime and 3rd party libraries more and more over time). > > Lessons that I got from that exercise are > 1. In order to work with a channel that could go away - close()d and/or > set to nil - you gotta have a second (control) channel for notifying that > this thing is dead or is on a life support (shutting down). "Full sync for > async!". Whole experience left very sad experience of writing TCP-like > "SEND-->ACK-->ACK of the ACK" versus simple RST (aka close/nil) over and > over. close() itself does not work well as a nice simple notification of > "done", it needs sync and be protected against "done-done". > Don't ever work with a channel that could "go away". Why are you trying to do that? Never, ever, ever call close() on a channel, unless you (the current goroutine) are the sole writer on the channel and you are done. close() is only ever a signal to readers on the channel. When using channels to signal events like "done", make sure you're only doing that close in one place, that will only be called once. If you REALLY have to have a done channel that can be signaled more than once, context.WithCancel() can help. The cancel function can be called more than once; subsequent calls do nothing, and ctx.Done() returns a channel that will be readable (closed) whenever the context is done. 2. No matter how much you check for nil even with synchronization > mechanism, you gotta get surprises at shutdown time! That's when things are > in the unstoppable avalanche of "I am going away!" broadcasts and close() > calls. Oh, and because shutdown is unstoppable, you cannot lock the house > for "lets wait for something!" party anymore. So, no close() then. > You should almost never be setting a chan variable to nil. It's only useful in certain specific circumstances. What are you actually trying to do? I feel like there's a fundamental misunderstanding here. 3. Unless you absolutely need to close a channel (so it serves as > notification mechanism for "range" loops), leave them alone and hope they > got decomposed by the worms of GC. > Why care whether GC will collect them? I mean, it will, if and when it needs to, as long as there are no remaining references. But what's the concern? Do you similarly avoid using slices because you have to "hope" the backing array will be GC'd? 4. close() itself is not protected by any barriers on entry, so even if you > checked it for nil or did a funky 'select' dance to check if it is closed > already, you gotta get surprises without full sync access to the channel > variable itself. > Right. Don't worry about it. Only close a channel once - when you know you're done writing to it. Full stop. Never write code that would potentially try to close a channel more than once. If you have multiple writers and you need to signal your readers when there is no more data, then keep track of your writers and once they are all done, close the channel. A sync.WaitGroup or errgroup.Group can be very useful in this regard. Also, again, don't set your chan variables to nil. The experience? Too much mental gymnastics over it. The (sad) conclusion? > It is not worth it, there are only so many hours of sleep left. So ... > > 5. I do not use channels -- too much hassle with controlling lifetime and > state of a channel itself. Slice+mutex is much easier to argue about (and - > bonus! - they can be regrown as needed). Yeah, we are losing nice "range" > idiom, and select here and there, and beauty of a channels-as-sync > mechanism. > > I have had a bit of experience with other languages, but not Go before, so > that was a learning experience for me. I wish that runtime would be more > forgiving on closing the closed, and closing the nilled the same way it > forgives nilling the nilled and nilling the closed. > I'm saddened that you have found this so frustrating. It's really not as complicated as it seems from your current point of view. If you show me some code that uses this Slice+mutex mechanism, I might be able to show you equivalent code using channels that's simpler and actually easier to reason about. It would almost certainly not involve setting any chan variables to nil. I am wondering how much shorter https://blog.golang.org/pipelines article > would be if "_ = close(c)" would be allowed, and used as "do not panic" way > of "I am done" notifications. > The second call to close() panics because it's very commonly an indicator of bad design. Similar to calling free() twice on the same block of memory. Thank you very much and sorry for the rant, > Andrey > > On Wednesday, March 13, 2019 at 11:21:45 AM UTC-6, rog wrote: >> >> It might be helpful to have a better idea of what you're actually trying >> to do here. >> Specifically, why do you feel the need to assign to c concurrently? >> >> On Wed, 13 Mar 2019 at 16:38, Andrey Tcherepanov < >> xnow4f...@sneakemail.com> wrote: >> >>> Thank you Peter, I know that - I wrote that example on purpose to be >>> undefined. >>> >>> To make it "well-defined" requires an enormous "fluff" which I am >>> mentioned and trying to avoid. >>> >>> Andrey >>> >>> On Wednesday, March 13, 2019 at 9:02:53 AM UTC-6, peterGo wrote: >>>> >>>> Andrey, >>>> >>>> Your program has a data race. The results are undefined. >>>> >>>> Data Race Detector https://golang.org/doc/articles/race_detector.html >>>> >>>> $ go run -race racer.go >>>> ================== >>>> WARNING: DATA RACE >>>> Write at 0x00000050dfa0 by goroutine 6: >>>> main.niller() >>>> /home/peter/gopath/src/racer.go:6 +0x3a >>>> >>>> Previous read at 0x00000050dfa0 by goroutine 5: >>>> main.closer() >>>> /home/peter/gopath/src/racer.go:10 +0x3a >>>> >>>> Goroutine 6 (running) created at: >>>> main.main() >>>> /home/peter/gopath/src/racer.go:18 +0xaa >>>> >>>> Goroutine 5 (finished) created at: >>>> main.main() >>>> /home/peter/gopath/src/racer.go:17 +0x92 >>>> ================== >>>> panic: close of closed channel >>>> >>>> goroutine 1 [running]: >>>> main.main() >>>> /home/peter/gopath/src/racer.go:23 +0xf4 >>>> exit status 2 >>>> $ >>>> >>>> Peter >>>> >>>> >>>> On Wednesday, March 13, 2019 at 2:40:30 AM UTC-4, Andrey Tcherepanov >>>> wrote: >>>>> >>>>> Hello fellow Go devs, >>>>> >>>>> I have a question that probably is a bit weird and obvious, but here >>>>> we go >>>>> >>>>> package main >>>>> >>>>> var c chan int >>>>> >>>>> func niller() { >>>>> c = nil >>>>> } >>>>> >>>>> func closer() { >>>>> close(c) >>>>> } >>>>> >>>>> func main() { >>>>> >>>>> c = make(chan int, 1) >>>>> >>>>> >>>>> go closer() >>>>> go niller() >>>>> >>>>> // checkpoint-1 >>>>> if c != nil { >>>>> // checkpoint-2 >>>>> close(c) >>>>> c = nil >>>>> } >>>>> } >>>>> >>>>> What are the guarantees (if any) that c, being non-nil at checkpoint-1 >>>>> will not become a nil at checkpoint-2? >>>>> >>>>> My take on it that there are none, and the code needs to be fully >>>>> synced around that close/nil. >>>>> But ... Is there any hard math theory around how `close()` MUST be >>>>> implemented in order to have some guarantees about concurrency and >>>>> consistency? >>>>> >>>>> (heavy IMHO warning) >>>>> Current implementation of close() starts with 2 checks and panics, and >>>>> the more I think of it, the less I am thrilled about both of them. They >>>>> both causing me nothing but headache by pretty much requiring 2 channels >>>>> everywhere code that could be much simpler with just 1 channel and no >>>>> fluff. Implementing this "fluff" is error prone and I would add it is not >>>>> a >>>>> junior dev task, on whom Go seems to be (quite controversially IMHO) is >>>>> focused on. >>>>> >>>>> So... Did anybody ever proposed a second close() variant that returns >>>>> an error instead of hard panic-ing inside? >>>>> >>>>> For example, if I have >>>>> >>>>> err := close(c) >>>>> >>>>> it will not panic, but if I use just >>>>> >>>>> close(c) >>>>> >>>>> all bets are off, just like in a current Go code? I think this would >>>>> be perfectly code-compatible with an "old" code, keeping Go1 compatibility >>>>> guarantee untouched. >>>>> >>>>> Thank you very much, >>>>> Andrey >>>>> >>>>> -- >>> You received this message because you are subscribed to the Google >>> Groups "golang-nuts" group. >>> To unsubscribe from this group and stop receiving emails from it, send >>> an email to golang-nuts...@googlegroups.com. >>> For more options, visit https://groups.google.com/d/optout. >>> >> -- > You received this message because you are subscribed to the Google Groups > "golang-nuts" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-nuts+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
