fix: use pointer receivers for piped protobuf models - #7174
Conversation
generated protobuf structs like applicationsyncstate embed a sync.mutex. because their methods were using value receivers, the lock state was being copied every time they were called or passed around. fix: - change methods on applicationsyncstate and applicationlivestateversion to use pointer receivers - update all caller sites in driftdetector and livestatestore to pass pointers - convert range loops in tests to index loops to avoid copying test structs Signed-off-by: srinivasr <sriniv4sreddy@gmail.com>
|
👋 Hi @srinivasr, welcome to PipeCD and thanks for opening your first pull request! We’re really happy to have you here Before your PR gets merged, please check a few important things below. Helpful resources
DCO Sign-offAll commits must include a In case you forget to sign-off your commit(s), follow these steps: For the last commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits: git rebase --signoff origin/master
git push --force-with-leaseRun checks locallyBefore pushing updates, please run: make checkThis runs the same checks as CI and helps catch issues early. 💬 Need help?If anything is unclear, feel free to ask in this PR or join us on the CNCF Slack in the #pipecd channel. Thanks for contributing to PipeCD! ❤️ |
✅ Deploy Preview for pipecd-site ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
i don’t think this branch compiles .. you should run |
Signed-off-by: srinivasr <sriniv4sreddy@gmail.com>
you're right, i missed a caller in builder.go when i was switching the models to use pointer receivers. i just pushed a fix and verified it builds cleanly locally now |
2 participants
What this PR does:
generated protobuf structs like applicationsyncstate embed a sync.mutex. because their methods were using value receivers, the lock state was being copied every time they were called or passed around (flagged by go vet).
fix:
Why we need it:
passing mutexes by value creates silent data races under concurrent execution because the copies do not block each other.
Which issue(s) this PR fixes:
Fixes # N/A
Does this PR introduce a user-facing change?: