fix: close the inspector client socket on disconnect#390
fix: close the inspector client socket on disconnect#390adrian-niculescu wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughUpdated diagnostic ChangesDiagnostic Message Improvements
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
NativeScript/inspector/InspectorServer.mm (2)
106-106: 💤 Low valueMinor formatting inconsistency in accept() error message.
Line 106 contains
NSLog(@"InspectorServer accept() failed;\n");. The embedded semicolon before the escape sequence is atypical (NSLog auto-appends newlines). Consider simplifying to:NSLog(@"InspectorServer accept() failed");This is a minor point and may be pre-existing; clarify intent if intentional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@NativeScript/inspector/InspectorServer.mm` at line 106, In the InspectorServer accept() error path, remove the extraneous semicolon and explicit newline from the NSLog call: replace the current NSLog(@"InspectorServer accept() failed;\n"); with a simpler NSLog(@"InspectorServer accept() failed"); so the message is formatted consistently (NSLog already appends a newline).
147-147: 💤 Low valueSend() error message uses inconsistent naming convention.
Line 147 uses
"InspectorServer::Send error"(with::C++ scope operator), while other messages use space-separated context. For consistency with lines 50, 59, 77, consider:NSLog(@"InspectorServer Send error: %s", strerror(error));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@NativeScript/inspector/InspectorServer.mm` at line 147, The log message in InspectorServer::Send currently uses an inconsistent C++ scope operator ("InspectorServer::Send error"); change the NSLog format string to match the project's space-separated convention (e.g., "InspectorServer Send error: %s") so it is consistent with other messages (see InspectorServer::Send and other NSLog calls around lines 50, 59, 77).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@NativeScript/inspector/InspectorServer.mm`:
- Line 106: In the InspectorServer accept() error path, remove the extraneous
semicolon and explicit newline from the NSLog call: replace the current
NSLog(@"InspectorServer accept() failed;\n"); with a simpler
NSLog(@"InspectorServer accept() failed"); so the message is formatted
consistently (NSLog already appends a newline).
- Line 147: The log message in InspectorServer::Send currently uses an
inconsistent C++ scope operator ("InspectorServer::Send error"); change the
NSLog format string to match the project's space-separated convention (e.g.,
"InspectorServer Send error: %s") so it is consistent with other messages (see
InspectorServer::Send and other NSLog calls around lines 50, 59, 77).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e99ec547-5a18-439f-8118-8d5dd0d38169
📒 Files selected for processing (1)
NativeScript/inspector/InspectorServer.mm
When the debugger client closes the connection, the header read handler fires with an empty buffer and returns without closing the dispatch_io channel, so the channel cleanup never runs and close(clientSocket) is never called. Each connect/disconnect leaks one fd and one channel. Close the channel on EOF so the existing cleanup runs, and label the NSLog messages so it's clear which stage failed.
adrian-niculescu
force-pushed
the
fix/inspector-socket-leak-on-disconnect
branch
from
9cf3a6a to
2a80d8a
Compare
Addressed the nitpick review comments, as well. |
1 participant
The inspector's TCP server leaks a socket on every client disconnect.
When the debugger client closes the connection, the header read handler fires with an empty buffer and just returns:
The dispatch_io channel is never closed there, so its cleanup handler never runs and the
close(clientSocket)inside it never happens. Each connect/disconnect leaks one fd and one channel, which adds up across a debugging session with reconnects. Closing the channel on EOF lets the existing cleanup run.I also relabeled the
NSLog(@"Error: %s", ...)lines. Right now accept, header read, body read, send, and channel cleanup all log the same generic string, so you can't tell which stage actually failed.Summary by CodeRabbit