fix(channel): return ChannelMember by value from GetMember#645
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe ChangesReturn Type Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The previous signature returned a pointer into s.members after releasing the RLock, so callers could read the struct while a writer holding s.mu mutated the same slice element. Returning by value forces the copy to happen under the read lock, matching what the lock semantics already implied.
e7a27ea to
0733719
Compare
Summary
GetMember in internal/channel/cursor.go:13 returned a pointer into s.members after releasing the RLock, so callers could read the struct while a writer holding s.mu mutated the same slice element. Pretty classic data race. The lock protected the lookup but not the dereference.
Fix
Return ChannelMember by value (and a zero value on miss) so the copy happens under the read lock. Callers that previously held the pointer now get a snapshot, which is what the lock semantics already implied.
Tests
bash scripts/test-go.shwith-raceagainst internal/channel passes; the previous shape would trip the race detector under concurrent MarkRead + GetMember, the new shape doesn't since no aliasing escapes the lock.Summary by CodeRabbit
Note: This release contains internal improvements with no user-facing changes.