There is a clear race in that code.  If both goroutines run simultaneously 
on different cores (or are context switched at just the "right" time - or 
just the "wrong" time, depending on how you look at it), then both will hit 
wg.Wait() when the waitgroup is empty, both will continue, both will call 
wg.Add(1), and then both will continue to do their job concurrently.

Hence if the objective of this code is to prevent two goroutines doing work 
concurrently, it's incorrect.  What you want for that is a mutex, as I 
think you've already realised.

If the objective is something else, then there will be a different way to 
achieve it, depending on what that objective is.

On Sunday, 22 January 2023 at 10:10:20 UTC bodqh...@gmail.com wrote:

> On 17.01.2023 10:57, Brian Candler wrote:
>
> You're supposed to do Add(1) -> Wait; and you'd normally wait once, in a 
> single goroutine. Typical pattern: 
>
> wg.Add(1)
> go func() {
>     defer wg.Done()
>     ... foo
> }
> wg.Add(1)
> go func() {
>     defer wg.Done()
>     ... bar
> }
> wg.Wait()  // wait for both goroutines to complete
>
> On Tuesday, 17 January 2023 at 04:56:44 UTC bodqh...@gmail.com wrote:
> The goroutine has a Wait -> Add(1) -> Done chain. 
>
> *Inside* a goroutine? That sounds problematic and racy, as you've found.
>
> - you're Waiting on the same waitgroup in multiple goroutines 
> concurrently?  Then when it counts to zero, multiple goroutines would be 
> able to start to run
>
> - you're  re-adding to a waitgroup even after it's counted down to zero?  
> That means that "wait" is non-deterministic (it may miss a count down to 
> zero and back up again).
>
> Well, I dived into the history of this code, and discovered that initially 
> Wait() was in a separate goroutine. Then another Wait() was added at the 
> start of this goroutine to prevent it from being run multiple times in a 
> row, and then the first separate Wait() call was removed.
>
> I checked with a simple MWE and seems that it should work as expected: 
> first Wait() before any Add() calls is a no-op, the second call of the same 
> goroutine is actually blocked until the first one finishes.
>
> @bq:12:01:11:/tmp/dl$ cat a.go
> package main
>
> import "fmt"
> import "sync"
> import "time"
>
> var wg sync.WaitGroup
>
> func main() {
>     go oink()
>     time.Sleep(1e9)
>     go oink()
>     time.Sleep(3e9)
> }
>
> func oink() {
>     wg.Wait()
>     wg.Add(1)
>     fmt.Printf("oink %v\n", time.Now())
>     time.Sleep(2e9)
>     wg.Done()
> }
> @bq:12:01:16:/tmp/dl$ go run a.go
> oink 2023-01-22 12:01:19.860472885 +0200 EET m=+0.000090026
> oink 2023-01-22 12:01:21.861451472 +0200 EET m=+2.001068963
>
> I still see no reasonable purpose to have the block between Wait() and 
> Add() to be outside of the critical section, so probably I'll rework it 
> with a mutex now anyway.
>
>
> It sounds to me like you need some other synchronization primitive, but I 
> don't know what it is you're trying to do.  Possibly a semaphore.
>
> This video is well worth watching, several times:
> https://www.youtube.com/watch?v=5zXAHh5tJqQ
>
> This opened my eyes to a very useful pattern:
> - have a one-item buffered channel
> - push a value into that channel
> - whenever you want to use it: pop it out, use it, and push the updated 
> value back in
>
> This is a very simple and clean way of doing what would otherwise require 
> a mutex to protect.
>
> Thanks, I was inspired with this technique several years ago too, but was 
> disillusioned soon as I found out that it works well only if the number of 
> calls is predictable (the number of dummy data put into a channel matches 
> the number of extracted ones for sure). Otherwise, the channels are pretty 
> fragile, and classic synchronization primitives are preferable; sometimes I 
> even used the latter to protect channels, as reading from empty channels 
> leads to deadlocks and writing to closed channels leads to panics.
>
>
> Maybe also useful:
> https://www.youtube.com/watch?v=f6kdp27TYZs
> https://www.youtube.com/watch?v=oV9rvDllKEg
>
> -- 
> You received this message because you are subscribed to a topic in the 
> Google Groups "golang-nuts" group.
> To unsubscribe from this topic, visit 
> https://groups.google.com/d/topic/golang-nuts/D67p8I5JUuk/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to 
> golang-nuts...@googlegroups.com.
> To view this discussion on the web visit 
> https://groups.google.com/d/msgid/golang-nuts/475d96a1-c628-42bd-9fcd-62fe66277599n%40googlegroups.com
>  
> <https://groups.google.com/d/msgid/golang-nuts/475d96a1-c628-42bd-9fcd-62fe66277599n%40googlegroups.com?utm_medium=email&utm_source=footer>
> .
>
>
>

-- 
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.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/golang-nuts/8cc5dad7-b715-4134-9fde-e0a60fa99155n%40googlegroups.com.

Reply via email to