-
Notifications
You must be signed in to change notification settings - Fork 2
Fix nil pointer dereference panic in GRPC server methods during cluster initialization #204
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix nil pointer dereference panic in GRPC server methods during cluster initialization #204
Conversation
Co-authored-by: bootjp <[email protected]>
adapter/grpc.go
Outdated
|
|
||
| res, err := r.coordinator.Dispatch(m) | ||
| if err != nil { | ||
| commitIndex := uint64(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
エラーのときは常に uint64(0) で良いのではないですか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're absolutely right! Simplified the error handling to always use uint64(0) for CommitIndex when there's an error, which is cleaner and more semantically correct. Applied to all four methods: RawPut, RawDelete, Put, and Delete. Commit: d056645
Co-authored-by: bootjp <[email protected]>
bootjp
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The application was experiencing flaky test failures with segmentation fault panics in the GRPC Delete method and related operations. The panic occurred when tests started before the Raft cluster was fully initialized:
Root Cause
When the Raft cluster is not ready and
coordinator.Dispatch()fails, it can return(nil, error). However, the error handling code was attempting to accessres.CommitIndexon the nil result:This affected four GRPC methods:
RawPut,RawDelete,Put, andDelete.Solution
Fixed the error handling to always use
uint64(0)for CommitIndex when errors occur, which is semantically correct since failed operations should not report a commit index:Testing
CommitIndex: 0when the coordinator fails to dispatch requestsThis fix makes the system more robust during cluster initialization and eliminates the flaky test failures caused by timing issues with Raft cluster readiness.
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.