CommandListener: stop busy-spinning on stdin EOF#230
Conversation
The console reader thread looped on std::getline(std::cin, ...) and `continue`d on empty lines. With a non-interactive stdin (a dedicated server in a Docker container with no TTY), std::cin is at EOF and getline returns immediately without blocking, so the loop pegs a full CPU core. Interactive runs block in getline and are unaffected, which is why this only showed up on the Linux/Docker server, not Windows. Break out of the loop when stdin is permanently closed (EOF); on any other transient stream failure, clear and back off instead of spinning.
WalkthroughThe command listener now handles failed stdin reads by exiting on EOF or clearing recoverable errors and retrying after a 100ms delay, preventing busy-spinning in non-interactive environments. ChangesCommand Listener Stdin Handling
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
code/framework/src/utils/command_listener.cpp (1)
22-31: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLGTM!
The EOF detection and 100ms backoff correctly prevent busy-spinning in non-interactive environments. The interaction with
Shutdown()is sound: if_runningis set tofalseduring the sleep, thecontinueloops back towhile (_running)and exits cleanly.One minor optional nit: the
100ms backoff is a magic number. Consider extracting it to a named constant (e.g.,static constexpr auto kRetryDelay = std::chrono::milliseconds(100);) for clarity and easier tuning.🤖 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 `@code/framework/src/utils/command_listener.cpp` around lines 22 - 31, Optionally replace the magic 100-millisecond delay in the command-listening loop with a named constant, such as kRetryDelay, defined near the relevant class or function, and use it in std::this_thread::sleep_for.
🤖 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 `@code/framework/src/utils/command_listener.cpp`:
- Around line 22-31: Optionally replace the magic 100-millisecond delay in the
command-listening loop with a named constant, such as kRetryDelay, defined near
the relevant class or function, and use it in std::this_thread::sleep_for.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b2f502bf-0759-4cf4-83df-4991a759fcff
📒 Files selected for processing (1)
code/framework/src/utils/command_listener.cpp
2 participants
The console reader thread looped on std::getline(std::cin, ...) and
continued on empty lines. With a non-interactive stdin (a dedicated server in a Docker container with no TTY), std::cin is at EOF and getline returns immediately without blocking, so the loop pegs a full CPU core. Interactive runs block in getline and are unaffected, which is why this only showed up on the Linux/Docker server, not Windows.Break out of the loop when stdin is permanently closed (EOF); on any other transient stream failure, clear and back off instead of spinning.
Summary by CodeRabbit