feat: add client max connection #136
Conversation
WalkthroughThe pull request introduces enhancements to client connection management across multiple components of the network infrastructure. The changes focus on implementing a configurable maximum client limit, adding methods to track and control client connections, and improving server initialization and connection handling. The modifications span several files including Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
src/net/thread_manager.h
Outdated
| @@ -145,6 +153,11 @@ void ThreadManager<T>::Stop() { | |||
| template <typename T> | |||
| requires HasSetFdFunction<T> | |||
| void ThreadManager<T>::OnNetEventCreate(int fd, const std::shared_ptr<Connection> &conn) { | |||
| if(getClientCount() >= netOptions_.GetMaxClients()){ | |||
| ::close(fd); | |||
| @@ -145,6 +153,11 @@ void ThreadManager<T>::Stop() { | |||
| template <typename T> | |||
| requires HasSetFdFunction<T> | |||
| void ThreadManager<T>::OnNetEventCreate(int fd, const std::shared_ptr<Connection> &conn) { | |||
| if(getClientCount() >= netOptions_.GetMaxClients()){ | |||
| ::close(fd); | |||
| return; | |||
|
代码格式化检查有问题, 请处理一下 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/net/thread_manager.h (2)
105-105: Consider using uint64_t for client count.For high-scale systems,
uint32_tmight overflow. Consider usinguint64_tto support a larger number of connections over time.- inline static std::atomic<uint32_t> clientCount_{0}; + inline static std::atomic<uint64_t> client_count_{0};🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
230-232: Add context to connection closure.Consider adding a reason for the closure to help with debugging and monitoring.
void ThreadManager<T>::CloseConnection(uint64_t connId) { - OnNetEventClose(connId, ""); + OnNetEventClose(connId, "connection closed by server"); }🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/kiwi.cc(1 hunks)src/net/event_server.h(1 hunks)src/net/net_options.h(1 hunks)src/net/thread_manager.h(6 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/kiwi.cc
- src/net/net_options.h
- src/net/event_server.h
🧰 Additional context used
🪛 GitHub Actions: kiwi
src/net/thread_manager.h
[error] File has clang-format style issues
🔇 Additional comments (1)
src/net/thread_manager.h (1)
Line range hint
1-380: Fix clang-format style issues.The pipeline reports formatting issues. Please run clang-format on the file to fix the style violations.
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
src/net/thread_manager.h
Outdated
| @@ -166,6 +185,7 @@ void ThreadManager<T>::OnNetEventCreate(int fd, const std::shared_ptr<Connection | |||
| } | |||
|
|
|||
| onCreate_(connId, t, conn->addr_); | |||
| clientCountIncrement(); | |||
There was a problem hiding this comment.
Fix potential race condition in client counting.
There's a race condition between checking the client count and incrementing it. Another thread could add a client between the check and increment, potentially exceeding the max limit.
template <typename T>
requires HasSetFdFunction<T>
void ThreadManager<T>::OnNetEventCreate(int fd, const std::shared_ptr<Connection> &conn) {
- if (getClientCount() >= netOptions_.GetMaxClients()) {
+ if (!client_count_.compare_exchange_strong(
+ expected,
+ expected + 1,
+ std::memory_order_seq_cst,
+ std::memory_order_seq_cst) ||
+ expected >= netOptions_.GetMaxClients()) {
INFO("Max client connections, refuse new connection fd: %d", fd);
std::string response = "-ERR max clients reached\r\n";
::send(fd, response.c_str(), response.size(), 0);
::close(fd);
return;
}
// ... rest of the connection setup ...
- clientCountIncrement(); // Remove this as we've already incremented atomically
}Also applies to: 225-225
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
src/net/thread_manager.h
Outdated
| uint32_t getClientCount() const { return clientCount_.load(); } | ||
|
|
||
| void clientCountIncrement() { clientCount_.fetch_add(1, std::memory_order_relaxed); } | ||
|
|
||
| void clientCountDecrement() { clientCount_.fetch_sub(1, std::memory_order_relaxed); } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Maintain consistent naming convention and review memory ordering.
-
Method names should follow the class's snake_case convention:
getClientCount→get_client_countclientCountIncrement→client_count_incrementclientCountDecrement→client_count_decrement
-
Consider using
std::memory_order_seq_cstinstead ofmemory_order_relaxedfor the atomic operations since the client count is used for critical decision-making inOnNetEventCreate.
- uint32_t getClientCount() const { return clientCount_.load(); }
+ uint32_t get_client_count() const { return client_count_.load(); }
- void clientCountIncrement() { clientCount_.fetch_add(1, std::memory_order_relaxed); }
+ void client_count_increment() { client_count_.fetch_add(1, std::memory_order_seq_cst); }
- void clientCountDecrement() { clientCount_.fetch_sub(1, std::memory_order_relaxed); }
+ void client_count_decrement() { client_count_.fetch_sub(1, std::memory_order_seq_cst); }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| uint32_t getClientCount() const { return clientCount_.load(); } | |
| void clientCountIncrement() { clientCount_.fetch_add(1, std::memory_order_relaxed); } | |
| void clientCountDecrement() { clientCount_.fetch_sub(1, std::memory_order_relaxed); } | |
| uint32_t get_client_count() const { return client_count_.load(); } | |
| void client_count_increment() { client_count_.fetch_add(1, std::memory_order_seq_cst); } | |
| void client_count_decrement() { client_count_.fetch_sub(1, std::memory_order_seq_cst); } |
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
src/net/thread_manager.h
Outdated
| if (getClientCount() >= netOptions_.GetMaxClients()) { | ||
| INFO("Max client connentions, refuse new connection fd:{%d}", fd); | ||
| std::string response = "-ERR max clients reached\r\n"; | ||
| ::send(fd, response.c_str(), response.size(), 0); | ||
| ::close(fd); | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and logging consistency.
- The log message format is inconsistent. Remove the curly braces around %d.
- Add error logging for connection closure.
- Handle potential errors from send/close system calls.
- INFO("Max client connentions, refuse new connection fd:{%d}", fd);
+ INFO("Max client connections, refuse new connection fd: %d", fd);
std::string response = "-ERR max clients reached\r\n";
- ::send(fd, response.c_str(), response.size(), 0);
- ::close(fd);
+ ssize_t sent = ::send(fd, response.c_str(), response.size(), 0);
+ if (sent < 0) {
+ ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno);
+ }
+ if (::close(fd) < 0) {
+ ERROR("Failed to close fd: %d, errno: %d", fd, errno);
+ }Also, fix the typo in "connentions" → "connections".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (getClientCount() >= netOptions_.GetMaxClients()) { | |
| INFO("Max client connentions, refuse new connection fd:{%d}", fd); | |
| std::string response = "-ERR max clients reached\r\n"; | |
| ::send(fd, response.c_str(), response.size(), 0); | |
| ::close(fd); | |
| return; | |
| } | |
| if (getClientCount() >= netOptions_.GetMaxClients()) { | |
| INFO("Max client connections, refuse new connection fd: %d", fd); | |
| std::string response = "-ERR max clients reached\r\n"; | |
| ssize_t sent = ::send(fd, response.c_str(), response.size(), 0); | |
| if (sent < 0) { | |
| ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno); | |
| } | |
| if (::close(fd) < 0) { | |
| ERROR("Failed to close fd: %d, errno: %d", fd, errno); | |
| } | |
| return; | |
| } |
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/net/thread_manager.h (1)
159-166:⚠️ Potential issueFix race condition and improve error handling.
- Race condition exists between checking and incrementing client count
- Error handling for send/close operations is missing
- Typo in log message: "connetions" → "connections"
Add error handling for system calls:
- ::send(fd, response.c_str(), response.size(), 0); - ::close(fd); + if (::send(fd, response.c_str(), response.size(), 0) < 0) { + ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno); + } + if (::close(fd) < 0) { + ERROR("Failed to close fd: %d, errno: %d", fd, errno); + }Also applies to: 188-188
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
🧹 Nitpick comments (1)
src/net/thread_manager.h (1)
225-225: Improve connection closure handling.
- Consider adding error handling for edge cases (e.g., closing already closed connections)
- The
CloseConnectionmethod could be enhanced to provide more value:
- Add validation for the connection state
- Return a status indicating success/failure
- Add logging for debugging purposes
void ThreadManager<T>::CloseConnection(uint64_t connId) { + { + std::shared_lock lock(mutex_); + auto iter = connections_.find(connId); + if (iter == connections_.end()) { + WARN("Attempting to close non-existent connection: %lu", connId); + return; + } + } + INFO("Closing connection: %lu", connId); OnNetEventClose(connId, ""); }Also applies to: 230-232
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/net/thread_manager.h(6 hunks)
🧰 Additional context used
🪛 GitHub Actions: kiwi
src/net/thread_manager.h
[error] File has clang-format style issues
🔇 Additional comments (2)
src/net/thread_manager.h (2)
93-98: Maintain consistent naming convention and review memory ordering.The implementation needs improvements in two areas:
- Method names should follow snake_case convention
- Memory ordering should be strengthened for critical operations
Also applies to: 105-105
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
Line range hint
1-380: Fix code formatting issues.The file has clang-format style issues reported by the CI pipeline. Please run clang-format on the file to fix the formatting issues.
🧰 Tools
🪛 GitHub Actions: kiwi
[error] File has clang-format style issues
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/net/thread_manager.h (2)
106-106: Consider instance-specific client counting.The static atomic counter is shared across all ThreadManager instances. This could lead to incorrect connection limits if multiple ThreadManager instances are used. Consider making it instance-specific or documenting this limitation.
🧰 Tools
🪛 GitHub Actions: kiwi
[error] Code formatting issues detected by clang-format. Issues include indentation and spacing in the OnNetEventCreate function.
166-166: Fix typo in log message."connetions" should be "connections".
- INFO("Max client connetions, refuse new connection fd: %d", fd); + INFO("Max client connections, refuse new connection fd: %d", fd);🧰 Tools
🪛 GitHub Actions: kiwi
[error] Code formatting issues detected by clang-format. Issues include indentation and spacing in the OnNetEventCreate function.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/kiwi.cc(1 hunks)src/net/event_server.h(1 hunks)src/net/net_options.h(2 hunks)src/net/thread_manager.h(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/net/net_options.h
- src/kiwi.cc
- src/net/event_server.h
🧰 Additional context used
🪛 GitHub Actions: kiwi
src/net/thread_manager.h
[error] Code formatting issues detected by clang-format. Issues include indentation and spacing in the OnNetEventCreate function.
🔇 Additional comments (3)
src/net/thread_manager.h (3)
93-97: Maintain consistent naming convention and strengthen memory ordering.The atomic operations need stronger memory ordering guarantees since they're used for critical connection management decisions.
🧰 Tools
🪛 GitHub Actions: kiwi
[error] Code formatting issues detected by clang-format. Issues include indentation and spacing in the OnNetEventCreate function.
236-236: LGTM!The client count decrement is correctly placed after all cleanup operations.
🧰 Tools
🪛 GitHub Actions: kiwi
[error] Code formatting issues detected by clang-format. Issues include indentation and spacing in the OnNetEventCreate function.
111-112: Fix indentation for better readability.The connection map declaration spans multiple lines with inconsistent indentation. This matches the clang-format issues reported in the pipeline.
Run this command to verify other similar formatting issues:
✅ Verification successful
Fix indentation for better readability
The connection map declaration should follow the predominant style in the codebase, where similar declarations are formatted more concisely. Consider using a single line or proper alignment if multiple lines are needed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find similar multi-line declarations with potential formatting issues rg -U "std::unordered_map<.*?\n.*?>" --type cppLength of output: 755
Script:
#!/bin/bash # Check for clang-format configuration fd -t f ".clang-format|.clang-format.yml" -x cat {}Length of output: 55
🧰 Tools
🪛 GitHub Actions: kiwi
[error] Code formatting issues detected by clang-format. Issues include indentation and spacing in the OnNetEventCreate function.
src/net/thread_manager.h
Outdated
| @@ -170,6 +196,7 @@ void ThreadManager<T>::OnNetEventCreate(int fd, const std::shared_ptr<Connection | |||
| } | |||
|
|
|||
| onCreate_(connId, t, conn->addr_); | |||
| client_count_increment(); | |||
There was a problem hiding this comment.
Move client count increment before onCreate_.
If onCreate_ throws an exception, the client count won't reflect the actual connection state. Move the increment before the callback to ensure accurate counting.
readThread_->AddNewEvent(connId, fd, BaseEvent::EVENT_READ);
if (writeThread_) {
writeThread_->AddNewEvent(connId, fd, BaseEvent::EVENT_NULL);
}
+ client_count_increment();
onCreate_(connId, t, conn->addr_);
- client_count_increment();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| client_count_increment(); | |
| readThread_->AddNewEvent(connId, fd, BaseEvent::EVENT_READ); | |
| if (writeThread_) { | |
| writeThread_->AddNewEvent(connId, fd, BaseEvent::EVENT_NULL); | |
| } | |
| client_count_increment(); | |
| onCreate_(connId, t, conn->addr_); |
🧰 Tools
🪛 GitHub Actions: kiwi
[error] Code formatting issues detected by clang-format. Issues include indentation and spacing in the OnNetEventCreate function.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/net/thread_manager.h (2)
93-98:⚠️ Potential issueUse stricter memory ordering for client count operations.
The relaxed memory ordering may not be appropriate for these operations as they are used in critical decision-making paths.
- uint32_t get_client_count() const { return clientCount_.load(); } + uint32_t get_client_count() const { return clientCount_.load(std::memory_order_seq_cst); } - void client_count_increment() { clientCount_.fetch_add(1, std::memory_order_relaxed); } + void client_count_increment() { clientCount_.fetch_add(1, std::memory_order_seq_cst); } - void client_count_decrement() { clientCount_.fetch_sub(1, std::memory_order_relaxed); } + void client_count_decrement() { clientCount_.fetch_sub(1, std::memory_order_seq_cst); }
195-195:⚠️ Potential issueMove client count increment before onCreate_.
If onCreate_ throws an exception, the client count won't reflect the actual connection state.
readThread_->AddNewEvent(connId, fd, BaseEvent::EVENT_READ); if (writeThread_) { writeThread_->AddNewEvent(connId, fd, BaseEvent::EVENT_NULL); } + client_count_increment(); onCreate_(connId, t, conn->addr_); - client_count_increment();
🧹 Nitpick comments (1)
src/net/thread_manager.h (1)
106-106: Document thread safety guarantees and consider testing implications.The static atomic counter could make unit testing difficult as its state persists across test cases. Consider:
- Adding documentation about thread safety guarantees
- Making the counter non-static and injecting it for better testability
- inline static std::atomic<uint32_t> clientCount_{0}; + // Thread-safe counter for tracking the number of active client connections + // across all ThreadManager instances. Protected by sequential consistency. + inline static std::atomic<uint32_t> clientCount_{0};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/net/net_options.h(2 hunks)src/net/thread_manager.h(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/net/net_options.h
🧰 Additional context used
🪛 GitHub Actions: kiwi
src/net/thread_manager.h
[error] 158-161: Code formatting issue: Incorrect line break in condition statement. The condition needs to be split across lines with proper alignment according to clang-format style.
🔇 Additional comments (2)
src/net/thread_manager.h (2)
111-112: LGTM! Good choice of data structures and synchronization.The use of
std::unordered_mapwithstd::shared_mutexprovides efficient concurrent access to connection data.
232-232: LGTM! Correct placement of client count decrement.The client count is decremented after all cleanup operations are complete, which is the appropriate location.
src/net/thread_manager.h
Outdated
| if (!clientCount_.compare_exchange_strong(expected, expected + 1, std::memory_order_seq_cst, | ||
| std::memory_order_seq_cst) || expected >= netOptions_.GetMaxClients()) { | ||
| INFO("Max client connetions, refuse new connection fd: %d", fd); | ||
| std::string response = "-ERR max clients reached\r\n"; | ||
| ssize_t sent = ::send(fd, response.c_str(), response.size(), 0); | ||
| if (sent < 0) { | ||
| ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno); | ||
| } | ||
| if (::close(fd) < 0) { | ||
| ERROR("Failed to close fd: %d, errno: %d", fd, errno); | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix code formatting issues.
The condition statement needs proper formatting according to clang-format style. Also, there's a typo in "connetions".
- if (!clientCount_.compare_exchange_strong(expected, expected + 1, std::memory_order_seq_cst,
- std::memory_order_seq_cst) || expected >= netOptions_.GetMaxClients()) {
- INFO("Max client connetions, refuse new connection fd: %d", fd);
+ if (!clientCount_.compare_exchange_strong(expected, expected + 1,
+ std::memory_order_seq_cst,
+ std::memory_order_seq_cst) ||
+ expected >= netOptions_.GetMaxClients()) {
+ INFO("Max client connections, refuse new connection fd: %d", fd);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (!clientCount_.compare_exchange_strong(expected, expected + 1, std::memory_order_seq_cst, | |
| std::memory_order_seq_cst) || expected >= netOptions_.GetMaxClients()) { | |
| INFO("Max client connetions, refuse new connection fd: %d", fd); | |
| std::string response = "-ERR max clients reached\r\n"; | |
| ssize_t sent = ::send(fd, response.c_str(), response.size(), 0); | |
| if (sent < 0) { | |
| ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno); | |
| } | |
| if (::close(fd) < 0) { | |
| ERROR("Failed to close fd: %d, errno: %d", fd, errno); | |
| } | |
| return; | |
| } | |
| if (!clientCount_.compare_exchange_strong(expected, expected + 1, | |
| std::memory_order_seq_cst, | |
| std::memory_order_seq_cst) || | |
| expected >= netOptions_.GetMaxClients()) { | |
| INFO("Max client connections, refuse new connection fd: %d", fd); | |
| std::string response = "-ERR max clients reached\r\n"; | |
| ssize_t sent = ::send(fd, response.c_str(), response.size(), 0); | |
| if (sent < 0) { | |
| ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno); | |
| } | |
| if (::close(fd) < 0) { | |
| ERROR("Failed to close fd: %d, errno: %d", fd, errno); | |
| } | |
| return; | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/net/thread_manager.h (1)
234-234: Move client count decrement before connection removal.To maintain accurate counting even if cleanup fails, move the decrement before erasing the connection.
+ client_count_decrement(); connections_.erase(iter); - client_count_decrement();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/net/thread_manager.h(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build_on_ubuntu
- GitHub Check: build_on_macos
🔇 Additional comments (4)
src/net/thread_manager.h (4)
11-11: LGTM! Well-structured additions for client counting.The new includes and static atomic counter are appropriately chosen for cross-thread client counting.
Also applies to: 22-23, 107-107
96-98: 🛠️ Refactor suggestionMaintain consistent naming and memory ordering.
Method names should follow the class's naming convention:
get_client_count→GetClientCountclient_count_decrement→ClientCountDecrementUse consistent memory ordering between increment and decrement operations.
- uint32_t get_client_count() const { return clientCount_.load(); } + uint32_t GetClientCount() const { return clientCount_.load(std::memory_order_seq_cst); } - void client_count_decrement() { clientCount_.fetch_sub(1, std::memory_order_relaxed); } + void ClientCountDecrement() { clientCount_.fetch_sub(1, std::memory_order_seq_cst); }Likely invalid or redundant comment.
161-175: 🛠️ Refactor suggestion
⚠️ Potential issueFix race condition in client counting.
The current implementation has a race condition between getting the count and performing the atomic operation.
- uint32_t expected = get_client_count(); - if (!clientCount_.compare_exchange_strong(expected, expected + 1, std::memory_order_seq_cst, - std::memory_order_seq_cst) || - expected >= netOptions_.GetMaxClients()) { + uint32_t expected = clientCount_.load(std::memory_order_seq_cst); + while (expected < netOptions_.GetMaxClients()) { + if (clientCount_.compare_exchange_weak(expected, expected + 1, + std::memory_order_seq_cst, + std::memory_order_seq_cst)) { + break; + } + } + if (expected >= netOptions_.GetMaxClients()) {Improve error response handling.
The current implementation doesn't handle partial sends or set timeouts.
- ssize_t sent = ::send(fd, response.c_str(), response.size(), 0); - if (sent < 0) { - ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno); - } + const char* buf = response.c_str(); + size_t remaining = response.size(); + while (remaining > 0) { + ssize_t sent = ::send(fd, buf, remaining, MSG_NOSIGNAL); + if (sent < 0) { + if (errno == EINTR) continue; + ERROR("Failed to send error response to fd: %d, errno: %d", fd, errno); + break; + } + buf += sent; + remaining -= sent; + }Likely invalid or redundant comment.
161-175: Verify GetMaxClients implementation and connection cleanup.Please ensure:
- The GetMaxClients method is properly implemented in NetOptions class
- All resources are properly cleaned up when connections are refused
✅ Verification successful
GetMaxClients implementation and connection cleanup are properly handled
The implementation is correct with:
- Proper getter/setter methods in NetOptions class
- Complete error handling and cleanup in connection refusal code
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check NetOptions implementation rg -A 5 "GetMaxClients|SetMaxClients" --type cpp # Check for potential resource leaks ast-grep --pattern 'class NetOptions { $$$ GetMaxClients() { $$$ } $$$ }'Length of output: 1433
Description
Changes related to client connection limits and management in the system.
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
close:#104