fix data race

[zhangxu19830126]

User description

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

issue #16141

What this PR does / why we need it:

fix data race

---

PR Type

Bug fix

---

Description

• Added a new disconnectedC channel to the clientSession struct to handle disconnections more gracefully.
• Modified the fetch function within startWriteLoop to return a boolean indicating if the session should be closed, improving control flow.
• Updated the startWriteLoop method to check the new disconnectedC channel and handle the boolean return value from fetch, ensuring proper session closure.
• Introduced a disconnected method to the clientSession struct to signal when a session is disconnected, preventing potential data races.

---

Changes walkthrough 📝

Relevant files

Bug fix

server.go Fix data race by adding disconnection handling and improving session management. pkg/common/morpc/server.go Added a disconnectedC channel to clientSession struct. Modified fetch function to return a boolean indicating if the session should be closed. Updated startWriteLoop to handle the new disconnectedC channel and the boolean return value from fetch. Introduced a disconnected method to clientSession to signal disconnection. +97/-85

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves multiple changes across the session management logic, including modifications to control flow and synchronization mechanisms. Understanding the context and ensuring that the changes resolve the data race without introducing new issues requires a moderate level of effort.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The fetch function now returns a boolean to indicate whether the session should be closed, but the handling of this return value might not cover all edge cases, especially under high load or unusual network conditions.
	Data Race: While the introduction of disconnectedC aims to handle disconnections better, the use of channels without proper synchronization might still lead to race conditions if not handled carefully in all scenarios where it's accessed.
🔒 Security concerns	No

[codiumai-pr-agent-pro]

PR-Agent was enabled for this repository. To continue using it, please link your git user with your CodiumAI identity here.

PR Code Suggestions ✨

Category	Suggestion	Score
Performance	Combine multiple select statements into a single select with multiple cases for better readability and efficiency The fetch function can be optimized by reducing the number of select statements. Instead of having multiple select statements, combine them into a single select with multiple cases. This will make the code more readable and potentially more efficient. pkg/common/morpc/server.go [315-327] select { - case <-ctx.Done(): - responses = nil - return true - case <-cs.ctx.Done(): - responses = nil - return true - case <-cs.disconnectedC: + case <-ctx.Done(), <-cs.ctx.Done(), <-cs.disconnectedC: responses = nil return true case resp, ok := <-cs.c: if ok { responses = append(responses, resp) } default: return false } Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies an opportunity to simplify and optimize the code by combining multiple select cases into one, which improves readability and efficiency.	8
	Add a small sleep duration in the loop to prevent high CPU usage In the startWriteLoop function, the fetch function is called in a loop. Consider adding a small sleep duration in the loop to prevent it from running too frequently and consuming too much CPU. pkg/common/morpc/server.go [334-429] for { closed := fetch() ... if closed { return } + time.Sleep(10 * time.Millisecond) // Adjust the duration as needed } Suggestion importance[1-10]: 5 Why: Adding a sleep to manage CPU usage could be beneficial in some scenarios, but it might not be the best approach for all use cases and could introduce unnecessary delays.	5
Possible bug	Add a check to ensure the channel is not closed before sending to it in the disconnected method The disconnected method in clientSession can be improved by adding a check to ensure that the channel is not closed before sending to it, to avoid potential panics. pkg/common/morpc/server.go [569-573] func (cs *clientSession) disconnected() { + select { + case <-cs.ctx.Done(): + return + default: + } select { case cs.disconnectedC <- struct{}{}: default: } } Suggestion importance[1-10]: 7 Why: This suggestion addresses a potential bug where sending on a closed channel could cause a panic. Adding a check before sending enhances robustness.	7
Maintainability	Create a helper function to handle setting responses to nil to reduce redundancy In the fetch function, the responses slice is set to nil in multiple places. Instead of repeating this line, consider creating a helper function to handle this logic, which will make the code cleaner and reduce redundancy. pkg/common/morpc/server.go [283-331] -responses = nil -return true +handleResponsesNil := func() bool { + responses = nil + return true +} +... +select { + case <-ctx.Done(): + return handleResponsesNil() + case <-cs.ctx.Done(): + return handleResponsesNil() + case <-cs.disconnectedC: + return handleResponsesNil() + case resp, ok := <-cs.c: + if ok { + responses = append(responses, resp) + } + default: + return false +} Suggestion importance[1-10]: 6 Why: The suggestion to create a helper function is valid for reducing redundancy and improving maintainability, though it's not critical for functionality.	6