Understanding Race Condition

[old bone]

I'm trying to understand what I'm doing wrong here...

     done := make(chan bool)
    stringsChan := make(chan string)
        //starts a makeshift timeout on processing
    go func() {
        time.AfterFunc(time.Nanosecond*500, func() {
            done <- true
        })
    }()
        // consumer read from channel and prints value
    go func() {
        for {
            select {
            case s := <-stringsChan:
                fmt.Println(s)
            case <-done:
                close(stringsChan)
                return
            }
        }
    }()

        //producer, writes data to channel till done signal.
    for i := range 200 {
        select {
        case <-done:
            close(stringsChan)
            break
        default:
            stringsChan <- fmt.Sprintf("%d", i)
        }
    }

The pattern i'm trying to recreate is having an operation being interrupted and have it gracefully end.

So I have a timer going and it writes to the done channel. I tried using two different done channels for the write and read operation, It feels like I should be writing the true flag twice since there's 2 reads but that also feels hacky.

[sand stirrup]

just close channel once in your AfterFunc and don't close it in consumer and producer.

[old bone]

Okay that helps, but then I'm still confused how that would work.

You have to reads on done(). aren't they both racing to see who gets data on the channel first?

PS. Doing the suggested changes gives me an

panic: send on closed channel [recovered]
    panic: send on closed channel

[sand stirrup]

check go tour

it says very explicitly consumer shouldn't close channel

and in general closing channel is optional

from https://go.dev/tour/concurrency/2 till https://go.dev/tour/concurrency/4

[narrow mural]

this code looks fine, you just need to remove the close
and add one after the loop

     done := make(chan bool)
    stringsChan := make(chan string)
        //starts a makeshift timeout on processing
    go func() {
        time.AfterFunc(time.Nanosecond*500, func() {
            done <- true
        })
    }()
        // consumer read from channel and prints value
    go func() {
        for {
            select {
            case s, ok := <-stringsChan:
                if !ok { return }
                fmt.Println(s)
            case <-done:
                return
            }
        }
    }()

        //producer, writes data to channel till done signal.
    for i := range 200 {
        select {
        case <-done:
            break
        default:
            stringsChan <- fmt.Sprintf("%d", i)
        }
    }
    close(stringsChan)

as @sand stirrup said closing channels is optional the GC already takes care of cleaning up channels.

The point of closing the channel is that it "sends" a close signal which is received by anyone forever (ok will be false).
So the consumer can stop itself to avoid eating resources.

[old bone]

Yeah, but there's a behavior change isn't it?

if I range over a channel that's not closed it'll never stop the loop won't it?

[narrow mural]

it will never stop indeed

but I doubt that what you want

as far of the example you showed, there is no change in behavior except it does not panic anymore

and does not leak resources but that not a "behavior"

[old bone]

@narrow mural when I run your code I still get a deadlock. fatal error: all goroutines are asleep - deadlock!

[narrow mural]

ah mb I missed it

you shouldn't be using default

[old bone]

Yeah, I understand. Just asking for clarity

[narrow mural]

        //producer, writes data to channel till done signal.
    for i := range 200 {
        select {
        case <-done:
            break
        case stringsChan <- fmt.Sprintf("%d", i):           
        }
    }
    close(stringsChan)

the problem with default is that it's not atomic, it first check done, then if done is not done it tries to send

also multiple goroutines are waiting on done

so you need to close done

right now you send, so only one of the two goroutines will receive it

c <- v one sender, one receiver
close(c) one sender, all the receivers, in other words broadcast

[old bone]

sweet. Finally got it

Okay so when closed, all channels (receivers) will get a signal that there is some data on there. Do I even need to write the true value or just close it?

[narrow mural]

I think you meant, all receivers

just close no need to write

[old bone]

yeah receiver

[narrow mural]

actually you don't need a type at all, since you are never sending anything

and so that what chan struct{} is for

it conveys no value, but the act to send a value, or more often close sends or broadcast a synchronisation signal

[modest tangle]

and/or use a context (which does that behind the scenes upon cancel())

[narrow mural]

true

[old bone]

is there an advantage of that of any type?

[narrow mural]

any is 16 bytes

empty is 0

[old bone]

Yeah, I've used cancel before... but i'm trying to learn concurrency and the limits / features of how channels work

[narrow mural]

take a look at the signature of context.Context.Done

[sand stirrup]

don't confuse struct{} and interface{}

[narrow mural]

you can use [0]literallyAnything too but kinda pointless
||there is one use for it in a vastly different context but that ultra niche||

[old bone]

Thank you, back to reading. I appreciate the feedback. I understand wg, channels, and mutex..but the nuance around using it isn't trivial.

[sand stirrup]

let me guess: is it one use about bumping struct alignment?

[modest tangle]

fair. you can open the implementation to see how it uses the channel for signaling but agreed sometimes duplicating for purpose of understanding is good

[narrow mural]

let me think.
Maybe I'm not sure

[old bone]

true.. also reading anything always makes sense and then writing it ...well does something else

[narrow mural]

does a zero sized array inherit the underlying's type allign ?

[sand stirrup]

yes

[narrow mural]

well there are two uses of [0]T

@sand stirrup this is to disallow ==
If you have a type where == is meaningless / wrong and you want to make sure people call your .Equal method (or don't use it at all in general)
You can add _ [0]func(), and because func() is incomparable so is [0]func() and so is your struct

anything incomparable would work but [0]func() is the convention

[old bone]

what about sync.Cond I was reading up on it but I have very rarely ever seen it used (Or mentioned). It seems like it does the broadcast and signal pattern that my done channel does as well.

[sand stirrup]

got it

[sand stirrup]

channels are cleaner way to do the same

[modest tangle]

I used it but it’s quite equivalent to a channel

[narrow mural]

You can implement channels with sync.Cond but the semantics are much harder to parse and very easy to write bug with.
It tends to be the third highest performance option after uncontended mutexes and uncontended atomics.

go's chan impl actually plainly just use the sync.Mutex under the hood and from memory it reimplements cond due to extra channel optimizations in the runtime or smth

[modest tangle]

eg https://github.com/fortio/terminal/blob/v0.27.0/interrupt.go#L175

I could just have shoved the buffer into a channel instead

[old bone]

it's a nifty pattern, but I'd rather understand one construct and get it well... the wg was the easiest to understand... now I'm starting to have flows that spans too many go routines and I'm trying to get what is going on better

Anyways back to more reading. Thank you all. I appreciate the feedback.

[modest tangle]

less reading more coding:)

[old bone]

one last question. Are go routines disabled somehow in go playgrounds?

https://go.dev/play/p/A0e4Tsm-lxR.

that same code works great locally... I can't believe that a 1ns timeout is too short

[modest tangle]

what do you expect ? to not reach this

198
199
--- PASS: TestDoneSignal (0.00s)
PASS

you're supposed to hold to the timer so it doesn't get GC'ed away

time.AfterFunc that returns a timer

[old bone]

ah, okay gotcha. Weird that my laptop behaves differently

[modest tangle]

probably your laptop has more threads so you get lucky and that timer thing gets ran right away

[old bone]

Here's another one. I have this bit of code. I realize that default isn't atomic, but I'm open to suggestion on how to use context correctly to terminate the operations once the Done signal is set. If I add a sleep to the doWork, it works better but that's obviously a bad pattern.

    doWork := func(ctx context.Context) chan string {
        stringChan := make(chan string)
        go func(ch chan string) {
            for {
                select {
                case <-ctx.Done():
                    return
                default:
                    intVal := fmt.Sprintf("%d", rand.Int())
                    ch <- intVal
                    //time.Sleep(time.Millisecond * 10)
                }

            }
        }(stringChan)

        return stringChan

    }

    ctx, cancel := context.WithCancel(context.Background())
    data := doWork(ctx)
    go func() any {
        timer := time.AfterFunc(time.Millisecond*100, func() {
            fmt.Println("Cancelling context")
            cancel()
            close(data)
        })
        return timer
    }()
    for item := range data {
        fmt.Println(item)
    }

[narrow mural]

you can select a channel write

so you select both the read from done and the write to the channel

[old bone]

Hmm, interesting...

[modest tangle]

if you click the link I sent above... you'll see an example (of selecting multiple things)