fix: Add mutex protection to prevent data races in datasource cache (#433)

### Problem
The `CachedDatasource` implementation contained a data race condition
where multiple goroutines could concurrently access and modify the
shared `cache` map without proper synchronization. This could lead to
unpredictable behavior, potential crashes, or data corruption in
high-concurrency environments.

The bug has been reproduced by adding a unit test for this scenario and
running the tests using the `-race` option to `go test` command as shown
below:

```sh
❯ go test -race ./pkg/...
?   	github.com/grafana/github-datasource/pkg	[no test files]
?   	github.com/grafana/github-datasource/pkg/dfutil	[no test files]
?   	github.com/grafana/github-datasource/pkg/errors	[no test files]
ok  	github.com/grafana/github-datasource/pkg/github	1.372s
ok  	github.com/grafana/github-datasource/pkg/github/client	(cached)
ok  	github.com/grafana/github-datasource/pkg/github/projects	1.572s
?   	github.com/grafana/github-datasource/pkg/httputil	[no test files]
?   	github.com/grafana/github-datasource/pkg/models	[no test files]
==================
WARNING: DATA RACE
Write at 0x00c00039a7b0 by goroutine 33:
  runtime.mapaccess2()
      /opt/homebrew/Cellar/go/1.24.1/libexec/src/internal/runtime/maps/runtime_swiss.go:117 +0x2dc
  github.com/grafana/github-datasource/pkg/plugin.(*CachedDatasource).saveCache()
      /Users/dgiagio/Devel/github-datasource/pkg/plugin/datasource_caching.go:89 +0x148
  github.com/grafana/github-datasource/pkg/plugin.TestWithCaching.func2()
      /Users/dgiagio/Devel/github-datasource/pkg/plugin/datasource_caching_test.go:32 +0xc4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.24.1/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.24.1/libexec/src/testing/testing.go:1851 +0x40

Previous write at 0x00c00039a7b0 by goroutine 32:
  runtime.mapaccess2()
      /opt/homebrew/Cellar/go/1.24.1/libexec/src/internal/runtime/maps/runtime_swiss.go:117 +0x2dc
  github.com/grafana/github-datasource/pkg/plugin.(*CachedDatasource).saveCache()
      /Users/dgiagio/Devel/github-datasource/pkg/plugin/datasource_caching.go:89 +0x148
  github.com/grafana/github-datasource/pkg/plugin.TestWithCaching.func1()
      /Users/dgiagio/Devel/github-datasource/pkg/plugin/datasource_caching_test.go:24 +0xc4
  testing.tRunner()
      /opt/homebrew/Cellar/go/1.24.1/libexec/src/testing/testing.go:1792 +0x180
  testing.(*T).Run.gowrap1()
      /opt/homebrew/Cellar/go/1.24.1/libexec/src/testing/testing.go:1851 +0x40
```


### Changes
This PR adds proper concurrency handling to the `CachedDatasource`
struct:
- Added a read-write mutex (`sync.RWMutex`) field to protect concurrent
access to the cache map
- Implemented appropriate locking in the following methods:
- `getCache`: Uses read lock (`RLock`/`RUnlock`) for read-only access
- `saveCache`: Uses write lock (`Lock`/`Unlock`) when modifying the
cache
    - `Cleanup`: Uses write lock when removing expired entries

### Testing
Added comprehensive unit tests in `datasource_caching_test.go` that
verify concurrent access safety:
- Concurrent reads from empty cache
- Concurrent writes to and reads from the cache
- Multiple concurrent reads from the cache

These tests ensure the cache operations remain thread-safe under
concurrent access patterns.

### Impact
This change improves stability and reliability of the GitHub datasource
plugin by eliminating potential race conditions, making it more robust
in high-concurrency environments.

---------

Co-authored-by: Zoltán Bedi <[email protected]>

Author: dgiagio

.changeset/lazy-crews-explain.md

@@ -0,0 +1,5 @@
+---
+'grafana-github-datasource': patch
+---
+
+fix: Add mutex protection to prevent data races in datasource cache

pkg/plugin/datasource_caching.go

@@ -6,6 +6,7 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"sync"
 	"time"
 
 	"github.com/grafana/github-datasource/pkg/dfutil"
@@ -46,7 +47,9 @@ var (
 // If there is no cached data to respond with, the CachedDatasource forwards the request to the Datasource
 type CachedDatasource struct {
 	datasource Datasource
-	cache      map[string]CachedResult
+
+	mu    sync.RWMutex // protects the cache map against concurrent access
+	cache map[string]CachedResult
 }
 
 func (c *CachedDatasource) getCache(req backend.DataQuery) (dfutil.Framer, error) {
@@ -55,6 +58,9 @@ func (c *CachedDatasource) getCache(req backend.DataQuery) (dfutil.Framer, error
 		return nil, err
 	}
 
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+
 	// Return cached value if it's there and it's not expired
 	res, ok := c.cache[key]
 	if ok {
@@ -77,6 +83,9 @@ func (c *CachedDatasource) saveCache(req backend.DataQuery, f dfutil.Framer, err
 		return nil, err
 	}
 
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
 	c.cache[key] = newCachedResult(f)
 	return f, err
 }
@@ -277,6 +286,9 @@ func getCacheKey(req backend.DataQuery) (string, error) {
 
 // Cleanup removes old cache keys
 func (c *CachedDatasource) Cleanup() {
+	c.mu.Lock()
+	defer c.mu.Unlock()
+
 	for k, v := range c.cache {
 		// If it's an expired value, then delete it
 		if v.ExpiresAt.Before(time.Now()) {

pkg/plugin/datasource_caching_test.go

@@ -0,0 +1,132 @@
+package plugin
+
+import (
+	"encoding/json"
+	"github.com/grafana/grafana-plugin-sdk-go/backend"
+	"github.com/grafana/grafana-plugin-sdk-go/data"
+	"github.com/stretchr/testify/assert"
+	"sync"
+	"testing"
+)
+
+// mockFramer is a struct implementing the Framer interface that returns predefined frames for testing purposes
+type mockFramer struct {
+	frames data.Frames
+}
+
+func (m mockFramer) Frames() data.Frames {
+	return m.frames
+}
+
+// Fixture for the test cases
+var dataQueryA = backend.DataQuery{JSON: json.RawMessage(`{"query": "A"}`)}
+var framesA = data.Frames{data.NewFrame("A", nil)}
+var dataQueryB = backend.DataQuery{JSON: json.RawMessage(`{"query": "B"}`)}
+var framesB = data.Frames{data.NewFrame("B", nil)}
+
+func TestWithCaching(t *testing.T) {
+	cachedDS := WithCaching(nil)
+
+	t.Run("read from empty cache concurrently", func(t *testing.T) {
+		var wg sync.WaitGroup
+
+		// Read goroutine 1
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.getCache(dataQueryA)
+			assert.Nil(t, f)
+			assert.ErrorIs(t, err, ErrNoValue)
+		}()
+
+		// Read goroutine 2
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.getCache(dataQueryA)
+			assert.Nil(t, f)
+			assert.ErrorIs(t, err, ErrNoValue)
+		}()
+
+		wg.Wait()
+	})
+
+	t.Run("write to and read from cache concurrently", func(t *testing.T) {
+		var wg sync.WaitGroup
+
+		// Write goroutine 1
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.saveCache(dataQueryA, mockFramer{frames: framesA}, nil)
+			assert.NoError(t, err)
+			assert.Equal(t, framesA, f.Frames())
+		}()
+
+		// Write goroutine 2
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.saveCache(dataQueryB, mockFramer{frames: framesB}, nil)
+			assert.NoError(t, err)
+			assert.Equal(t, framesB, f.Frames())
+		}()
+
+		// Wait for writing goroutines
+		wg.Wait()
+
+		// Read goroutine 1
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.getCache(dataQueryA)
+			assert.NoError(t, err)
+			assert.Equal(t, framesA, f.Frames())
+		}()
+
+		// Read goroutine 2
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.getCache(dataQueryB)
+			assert.NoError(t, err)
+			assert.Equal(t, framesB, f.Frames())
+		}()
+
+		// Wait for reading goroutines
+		wg.Wait()
+	})
+
+	t.Run("read from the cache concurrently", func(t *testing.T) {
+		var wg sync.WaitGroup
+
+		// Read goroutine 1
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.getCache(dataQueryA)
+			assert.NoError(t, err)
+			assert.Equal(t, framesA, f.Frames())
+		}()
+
+		// Read goroutine 2
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+
+			f, err := cachedDS.getCache(dataQueryB)
+			assert.NoError(t, err)
+			assert.Equal(t, framesB, f.Frames())
+		}()
+
+		// Wait for reading goroutines
+		wg.Wait()
+	})
+}