Is this bad concurrency?

181 views
Skip to first unread message

bill....@talentinc.com

unread,
Aug 18, 2017, 2:13:36 PM8/18/17
to golang-nuts
Hi,


I have two long running things to do, I'd like to do them in parallel, then sync up at the end. I've written it as a function which gets one thing done, and which backgrounds the second thing with a goroutine. Thing one has a lot of reasons to exit early, and I need to signal those to thing two so that the go routine can exit cleanly and not create a goroutine leak. Standard fare, usually solved by introducing a channel and a select statement. I want to signal done for early exits, including panics,  so I thought I would write to the done channel in a defer block. That caused a problem, because in the happy case, both the foreground and the background routines are finished, the done channel has no reader, so writing to it in the defer block causes "fatal error: all goroutines are asleep - deadlock!" So I introduced a new boolean variable ("clean_finish") that is false until the background job completes, and which is tested in the defer block before writing to the done channel. Is there anything wrong with my approach? You can see the code in the playground link above, or down below.


package main

import (
"sync/atomic"
"fmt"
"time"
)

func main() {
ch := make(chan int)
done := make(chan struct{})
var clean_finish uint32
defer func() {
fin := atomic.LoadUint32(&clean_finish)
if fin != 1 {
done <- struct{}{}
}
}()
go func() {
defer func() {
fmt.Println("we're done here")
}()
select {
case <-time.After(time.Second * 1):
ch <- 1
atomic.StoreUint32(&clean_finish, 1)
case <- done:
fmt.Println("early exit")
}
}()
panic("bye") // comment this out to see normal behavior
n := <- ch
fmt.Printf("n: %v\n", n)
}

John Souvestre

unread,
Aug 18, 2017, 2:16:00 PM8/18/17
to golan...@googlegroups.com

You might want to consider using a sync.WaitGroup.  It’s for such situations.

 

John

    John Souvestre - New Orleans LA

--
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.

Tamás Gulácsi

unread,
Aug 18, 2017, 2:40:39 PM8/18/17
to golang-nuts

bill....@talentinc.com

unread,
Aug 18, 2017, 5:19:26 PM8/18/17
to golang-nuts
Thanks to both for replying. Both pieces of advice cover the use case of waiting for all goroutines to complete before exiting. I have the opposite problem. I want to end all goroutines if I exit. Is there anything in the sync package for that?

On Friday, August 18, 2017 at 2:40:39 PM UTC-4, Tamás Gulácsi wrote:
See golang.org/x/sync/errgroup.WithContext.

Josh Humphries

unread,
Aug 18, 2017, 5:36:17 PM8/18/17
to bill....@talentinc.com, golang-nuts
There's not a way to terminate a goroutine. But you can create a context with cancellation. Each goroutine would need to periodically check context.Err() and exit on its own.

When you exit, you cancel the context. You could combine this with a WaitGroup in order to not exit until all other goroutines have completed (e.g. cancel the context and then await, provided you've added to the wait group for each task and each one marks their work as done on completion).

----
Josh Humphries
jh...@bluegosling.com

--
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+unsubscribe@googlegroups.com.

Carl Mastrangelo

unread,
Aug 18, 2017, 9:59:51 PM8/18/17
to golang-nuts
FYI:   It is possible for `done` to never be sent to.  For example consider the following sequence:

goroutine:   ch <- 1
main: <- 1
main: run defer
main: fin = atomic load
goroutine: atomic store
main: not 1, return

bill....@talentinc.com

unread,
Aug 21, 2017, 10:53:59 AM8/21/17
to golang-nuts
That is ok, though, because the goroutine is exiting anyway. But I take your point, it's still a race condition -- two atomic operations in a row aren't helping. It's the same situation as testing (but not acquiring) a mutex before entering a function.
Reply all
Reply to author
Forward
0 new messages