[bug]: sync.Map passed by value causing lock copy violations detected by go vet
- Dominant language
- Go
- Stars
- 18.5k
- Forks
- 2.4k
- Avg merge
- 1d 4h
- Merged PRs (30d)
- 101
Description
### 👀 Is there an existing issue for this?
- [x] I have searched and didn't find similar issue
### 👍 Current behavior
Running `go vet ./...` reveals that `sync.Map` is being passed by value in the telemetry and CLI provider code. This copies the internal mutex (`sync.noCopy`) and can lead to race conditions, deadlocks, and undefined behavior in concurrent operations.
**Expected behavior:** `sync.Map` should be passed by pointer (`*sync.Map`) to avoid copying the internal lock state.
**Actual behavior:** `sync.Map` is passed by value in struct fields and function parameters, triggering `go vet` warnings about lock copying.
### 👟 Steps to Replicate
1. Clone the Keploy repository
2. Navigate to the project root directory
3. Run the following command:
```bash
go vet ./...
### 📜 Logs (if any)
# go.keploy.io/server/v3/pkg/platform/telemetry
pkg/platform/telemetry/telemetry.go:34:43: NewTelemetry passes lock by value: go.keploy.io/server/v3/pkg/platform/telemetry.Options contains sync.Map contains sync.noCopy
pkg/platform/telemetry/telemetry.go:39:19: literal copies lock value from opt.GlobalMap: sync.Map contains sync.noCopy
# go.keploy.io/server/v3/cli/provider
cli/provider/service.go:38:19: literal copies lock value from TeleGlobalMap: sync.Map contains sync.noCopy
### 💻 Operating system
Linux
### 🧾 System Info (`uname -a`)
Linux Anand 5.15.167.4-microsoft-standard-WSL2 #1 SMP Tue Nov 5 00:21:55 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
### 📦 OS Release Info (`cat /etc/os-release`)
NAME="Ubuntu"
VERSION="22.04.5 LTS (Jammy Jellyfish)"
ID=ubuntu
ID_LIKE=debian
PRETTY_NAME="Ubuntu 22.04.5 LTS"
VERSION_ID="22.04"
### 🐳 Docker Info (if applicable)
Not used
### 🧱 Your Environment
Built from source (main branch)
Running on Ubuntu 22.04 via WSL2
### 🎲 Version
keploy server v3 (from go.mod: go.keploy.io/server/v3)
Go version: go1.22.1 linux/amd64
### 📦 Repository
keploy
### 🤔 What use case were you trying? (optional)
"I was performing routine static analysis and code health checks using go vet to ensure that the project follows Go's concurrency best practices. I noticed these warnings which indicate potential instability in how telemetry data and global maps are handled across different goroutines."
### 🛠️ Proposed Fix
To resolve these go vet violations, we need to ensure that sync.Map is never copied by value. I propose the following changes:
1. In pkg/platform/telemetry/telemetry.go:
Update the Options struct to use GlobalMap *sync.Map.
Update the Telemetry struct to use GlobalMap *sync.Map.
Update the NewTelemetry constructor to accept and assign the pointer.
2. In cli/provider/service.go:
Pass the address of TeleGlobalMap (i.e., &TeleGlobalMap) when initializing the telemetry.Options struct.
I have already tested these changes locally, and go vet ./... passes successfully after the fix. I would like to submit a Pull Request for this!
Contributor guide
Research direction
Start with pkg/platform/telemetry/telemetry.go, then inspect cli/provider/service.go where the reported sync.Map copies occur. Run go vet ./... before and after the change, and confirm it completes without lock-copy warnings while telemetry and CLI provider behavior remains intact.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- backend, cli
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 55/100