From fe2103bfc8a71e10039dff1489dd1287e5791741 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jean-Philippe=20Qu=C3=A9m=C3=A9ner?=
 <JohnnyQQQQ@users.noreply.github.com>
Date: Mon, 5 Jun 2023 11:21:11 +0200
Subject: [PATCH] Util: Fix panic when generating UIDs concurrently (#69476)

---
 pkg/util/shortid_generator.go      |  8 ++++++++
 pkg/util/shortid_generator_test.go | 20 ++++++++++++++++++++
 2 files changed, 28 insertions(+)

diff --git a/pkg/util/shortid_generator.go b/pkg/util/shortid_generator.go
index ba4ad4d9745..03107c70fac 100644
--- a/pkg/util/shortid_generator.go
+++ b/pkg/util/shortid_generator.go
@@ -3,6 +3,7 @@ package util
 import (
 	"math/rand"
 	"regexp"
+	"sync"
 	"time"
 
 	"github.com/google/uuid"
@@ -12,6 +13,11 @@ var uidrand = rand.New(rand.NewSource(time.Now().UnixNano()))
 var alphaRunes = []rune("abcdefghijklmnopqrstuvwxyz")
 var hexLetters = []rune("abcdef")
 
+// We want to protect our number generator as they are not thread safe. Not using
+// the mutex could result in panics in certain cases where UIDs would be generated
+// at the same time.
+var mtx sync.Mutex
+
 // Legacy UID pattern
 var validUIDPattern = regexp.MustCompile(`^[a-zA-Z0-9\-\_]*$`).MatchString
 
@@ -30,6 +36,8 @@ func IsShortUIDTooLong(uid string) bool {
 // it is guaranteed to have a character as the first letter
 // This UID will be a valid k8s name
 func GenerateShortUID() string {
+	mtx.Lock()
+	defer mtx.Unlock()
 	uid, err := uuid.NewRandom()
 	if err != nil {
 		// This should never happen... but this seems better than a panic
diff --git a/pkg/util/shortid_generator_test.go b/pkg/util/shortid_generator_test.go
index b582520a825..fe7ad937e1c 100644
--- a/pkg/util/shortid_generator_test.go
+++ b/pkg/util/shortid_generator_test.go
@@ -1,6 +1,7 @@
 package util
 
 import (
+	"sync"
 	"testing"
 
 	"github.com/google/uuid"
@@ -16,6 +17,25 @@ func TestAllowedCharMatchesUidPattern(t *testing.T) {
 	}
 }
 
+// Run with "go test -race -run ^TestThreadSafe$ github.com/grafana/grafana/pkg/util"
+func TestThreadSafe(t *testing.T) {
+	// This test was used to showcase the bug, unfortunately there is
+	// no way to enable the -race flag programmatically.
+	t.Skip()
+	// Use 1000 go routines to create 100 UIDs each at roughly the same time.
+	var wg sync.WaitGroup
+	for i := 0; i < 1000; i++ {
+		go func() {
+			for ii := 0; ii < 100; ii++ {
+				_ = GenerateShortUID()
+			}
+			wg.Done()
+		}()
+		wg.Add(1)
+	}
+	wg.Wait()
+}
+
 func TestRandomUIDs(t *testing.T) {
 	for i := 0; i < 100; i++ {
 		v := GenerateShortUID()