From dad78f31f329ad03398c163ae1c198d85669b758 Mon Sep 17 00:00:00 2001
From: Joe Tsai <joetsai@digital-static.net>
Date: Thu, 9 Mar 2023 12:04:38 -0800
Subject: [PATCH] syncs: add WaitGroup wrapper (#7481)

The addition of WaitGroup.Go in the standard library has been
repeatedly proposed and rejected.
See golang/go#18022, golang/go#23538, and golang/go#39863

In summary, the argument for WaitGroup.Go is that it avoids bugs like:

	go func() {
		wg.Add(1)
		defer wg.Done()
		...
	}()

where the increment happens after execution (not before)
and also (to a lesser degree) because:

	wg.Go(func() {
		...
	})

is shorter and more readble.

The argument against WaitGroup.Go is that the provided function
takes no arguments and so inputs and outputs must closed over
by the provided function. The most common race bug for goroutines
is that the caller forgot to capture the loop iteration variable,
so this pattern may make it easier to be accidentally racy.
However, that is changing with golang/go#57969.

In my experience the probability of race bugs due to the former
still outwighs the latter, but I have no concrete evidence to prove it.

The existence of errgroup.Group.Go and frequent utility of the method
at least proves that this is a workable pattern and
the possibility of accidental races do not appear to
manifest as frequently as feared.

A reason *not* to use errgroup.Group everywhere is that there are many
situations where it doesn't make sense for the goroutine to return an error
since the error is handled in a different mechanism
(e.g., logged and ignored, formatted and printed to the frontend, etc.).
While you can use errgroup.Group by always returning nil,
the fact that you *can* return nil makes it easy to accidentally return
an error when nothing is checking the return of group.Wait.
This is not a hypothetical problem, but something that has bitten us
in usages that was only using errgroup.Group without intending to use
the error reporting part of it.

Thus, add a (yet another) variant of WaitGroup here that
is identical to sync.WaitGroup, but with an extra method.

Signed-off-by: Joe Tsai <joetsai@digital-static.net>
---
 syncs/syncs.go | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/syncs/syncs.go b/syncs/syncs.go
index 4d2891e3a..52fcad220 100644
--- a/syncs/syncs.go
+++ b/syncs/syncs.go
@@ -217,3 +217,19 @@ func (m *Map[K, V]) Range(f func(key K, value V) bool) {
 		}
 	}
 }
+
+// WaitGroup is identical to [sync.WaitGroup],
+// but provides a Go method to start a goroutine.
+type WaitGroup struct{ sync.WaitGroup }
+
+// Go calls the given function in a new goroutine.
+// It automatically increments the counter before execution and
+// automatically decrements the counter after execution.
+// It must not be called concurrently with Wait.
+func (wg *WaitGroup) Go(f func()) {
+	wg.Add(1)
+	go func() {
+		defer wg.Done()
+		f()
+	}()
+}