github/OpenHands
1
0
Fork 0
mirror of https://github.com/OpenHands/OpenHands.git synced 2025-12-26 13:52:43 +08:00
Code Issues Packages Projects Releases Wiki Activity

Simplify Ctrl+C handling implementation

Browse Source 
- Replace complex ProcessSignalHandler with SimpleSignalHandler
  - Direct signal handling in main process instead of queue communication
  - Simple Ctrl+C counting with immediate force kill on second press
  - Reset functionality to clear count when starting new operations

- Replace ProcessBasedConversationRunner with SimpleProcessRunner
  - Minimal multiprocessing - only process_message runs in subprocess
  - Direct method calls for status, settings, and other operations
  - No unnecessary queue communication

- Update agent_chat.py to use simplified components
  - Reset Ctrl+C count when starting new message processing
  - Direct method calls for commands that don't need process isolation
  - Cleaner error handling and resource cleanup

- Update simple_main.py imports

Fixes issues where second Ctrl+C wouldn't register properly due to
complex queue-based communication and race conditions.

Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
openhands 2025-11-03 16:46:43 +00:00
parent 5351702d3a
commit 6ecaca5b3c
5 changed files with 325 additions and 84 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

88
openhands-cli/CTRL_C_IMPROVEMENTS.md Normal file
View File

@ -0,0 +1,88 @@
# Ctrl+C Handling Improvements
## Summary
Simplified the overly complex Ctrl+C handling implementation in the OpenHands CLI to make it more reliable and easier to understand.
## Problems Addressed
1. **Second Ctrl+C not registering properly** - The original implementation had complex queue-based communication that could miss signals
2. **Overly complex multiprocessing** - Many methods were unnecessarily wrapped in separate processes
3. **No reset of Ctrl+C count** - The count wasn't reset when starting new message processing
4. **Unnecessary queue communication** - Status and settings methods didn't need separate processes
## Solution
### 1. Simplified Signal Handler (`simple_signal_handler.py`)
- **Direct signal handling** in the main process instead of complex queue communication
- **Simple Ctrl+C counting** with immediate force kill on second press within 3 seconds
- **Clear process management** with direct process termination
- **Reset functionality** to clear count when starting new operations
Key features:
- First Ctrl+C: Graceful termination (SIGTERM)
- Second Ctrl+C (within 3 seconds): Force kill (SIGKILL)
- Automatic count reset after 3 seconds
- Manual count reset via `reset_count()`
### 2. Simplified Process Runner (`simple_process_runner.py`)
- **Minimal multiprocessing** - Only the `process_message` method runs in a subprocess
- **Direct method calls** for status, settings, and other operations
- **Simple API** with clear process lifecycle management
- **No queue communication** for methods that don't need it
Key features:
- `process_message()`: Runs in subprocess for isolation
- `get_status()`, `get_settings()`, etc.: Run directly in main process
- `cleanup()`: Simple process termination
- `current_process` property for signal handler integration
### 3. Updated Main CLI (`agent_chat.py`)
- **Simplified imports** using the new signal handler and process runner
- **Reset Ctrl+C count** when starting new message processing
- **Direct method calls** for commands that don't need process isolation
- **Cleaner error handling** and resource cleanup
## Files Modified
### New Files
- `openhands_cli/simple_signal_handler.py` - Simplified signal handling
- `openhands_cli/simple_process_runner.py` - Minimal process wrapper
### Modified Files
- `openhands_cli/agent_chat.py` - Updated to use simplified components
- `openhands_cli/simple_main.py` - Updated imports
### Test Files
- `test_basic_signal.py` - Basic signal handler test
- `manual_test_ctrl_c.py` - Manual Ctrl+C testing
## Key Improvements
1. **Reliability**: Direct signal handling eliminates race conditions
2. **Simplicity**: Removed complex queue-based communication
3. **Performance**: Most operations run directly in main process
4. **Maintainability**: Clear, simple code that's easy to understand
5. **User Experience**: Consistent Ctrl+C behavior with immediate force kill option
## Testing
The implementation includes test scripts to verify:
- Basic signal handler functionality
- Ctrl+C counting and reset behavior
- Process termination (graceful and force)
- Integration with the CLI
## Usage
The simplified implementation maintains the same external API:
- First Ctrl+C: Attempts graceful pause/termination
- Second Ctrl+C (within 3 seconds): Force kills the process immediately
- Count resets automatically or when starting new operations
## Migration
The changes are backward compatible with the existing CLI interface. The complex `ProcessSignalHandler` and `ProcessBasedConversationRunner` classes are replaced with simpler equivalents that provide the same functionality with better reliability.

126
openhands-cli/openhands_cli/agent_chat.py
View File

@ -17,8 +17,8 @@ from prompt_toolkit import print_formatted_text
from prompt_toolkit.formatted_text import HTML
from openhands_cli.runner import ConversationRunner
from openhands_cli.process_runner import ProcessBasedConversationRunner
from openhands_cli.signal_handler import ProcessSignalHandler
from openhands_cli.simple_process_runner import SimpleProcessRunner
from openhands_cli.simple_signal_handler import SimpleSignalHandler
from openhands_cli.setup import (
MissingAgentSpec,
setup_conversation,
@ -97,13 +97,13 @@ def run_cli_entry(resume_conversation_id: str | None = None) -> None:
# Track session start time for uptime calculation
session_start_time = datetime.now()
# Create process-based conversation runner and signal handler
process_runner = None
signal_handler = ProcessSignalHandler()
# Create simple signal handler and session
signal_handler = SimpleSignalHandler()
signal_handler.install()
session = get_session_prompter()
# Install signal handler for Ctrl+C management
signal_handler.install_handler()
# Create simple process runner
process_runner = SimpleProcessRunner(str(conversation_id), setup_conversation)
try:
# Main chat loop
@ -151,23 +151,17 @@ def run_cli_entry(resume_conversation_id: str | None = None) -> None:
elif command == '/new':
try:
# Stop existing process runner if running
# Clean up existing process runner
if process_runner:
process_runner.stop()
process_runner.cleanup()
# Start a fresh conversation with new process runner
process_runner = ProcessBasedConversationRunner(conversation_id, setup_conversation)
if process_runner.start():
signal_handler.set_conversation_process(process_runner.process)
display_welcome(conversation_id, resume=False)
print_formatted_text(
HTML('<green>✓ Started fresh conversation</green>')
)
else:
print_formatted_text(
HTML('<red>Failed to start fresh conversation</red>')
)
process_runner = None
# Create fresh conversation with new process runner
conversation_id = uuid.uuid4()
process_runner = SimpleProcessRunner(str(conversation_id), setup_conversation)
display_welcome(conversation_id, resume=False)
print_formatted_text(
HTML('<green>✓ Started fresh conversation</green>')
)
continue
except Exception as e:
print_formatted_text(
@ -180,74 +174,42 @@ def run_cli_entry(resume_conversation_id: str | None = None) -> None:
continue
elif command == '/status':
if process_runner:
status = process_runner.get_status()
if status:
# Display status using the retrieved information
print_formatted_text(HTML(f'<yellow>Agent Status:</yellow> {status.get("agent_status", "Unknown")}'))
print_formatted_text(HTML(f'<yellow>Confirmation Mode:</yellow> {"Enabled" if status.get("confirmation_mode") else "Disabled"}'))
print_formatted_text(HTML(f'<yellow>Process Alive:</yellow> {process_runner.is_alive()}'))
else:
print_formatted_text(HTML('<red>Unable to get conversation status</red>'))
else:
print_formatted_text(HTML('<yellow>No active conversation</yellow>'))
status = process_runner.get_status()
print_formatted_text(HTML(f'<yellow>Conversation ID:</yellow> {status["conversation_id"]}'))
print_formatted_text(HTML(f'<yellow>Agent State:</yellow> {status.get("agent_state", "Unknown")}'))
print_formatted_text(HTML(f'<yellow>Process Running:</yellow> {status["is_running"]}'))
continue
elif command == '/confirm':
if process_runner:
result = process_runner.toggle_confirmation_mode()
if result:
print_formatted_text(HTML(f'<yellow>{result}</yellow>'))
else:
print_formatted_text(HTML('<red>Failed to toggle confirmation mode</red>'))
else:
print_formatted_text(HTML('<yellow>No active conversation</yellow>'))
result = process_runner.toggle_confirmation_mode()
mode_text = "Enabled" if result else "Disabled"
print_formatted_text(HTML(f'<yellow>Confirmation mode: {mode_text}</yellow>'))
continue
elif command == '/resume':
if not process_runner:
print_formatted_text(
HTML('<yellow>No active conversation running...</yellow>')
)
continue
status = process_runner.get_status()
if not status:
print_formatted_text(
HTML('<red>Unable to get conversation status</red>')
)
continue
agent_status = status.get("agent_status")
if not (
agent_status == AgentExecutionStatus.PAUSED
or agent_status == AgentExecutionStatus.WAITING_FOR_CONFIRMATION
):
print_formatted_text(
HTML('<red>No paused conversation to resume...</red>')
)
continue
# Resume without new message
if process_runner.resume():
print_formatted_text(HTML('<green>Conversation resumed</green>'))
else:
print_formatted_text(HTML('<red>Failed to resume conversation</red>'))
try:
process_runner.resume()
print_formatted_text(HTML('<green>Agent resumed</green>'))
except Exception as e:
print_formatted_text(HTML(f'<red>Failed to resume: {e}</red>'))
continue
# Create process runner if it doesn't exist
if not process_runner:
process_runner = ProcessBasedConversationRunner(conversation_id, setup_conversation)
if not process_runner.start():
print_formatted_text(HTML('<red>Failed to start conversation process</red>'))
continue
signal_handler.set_conversation_process(process_runner.process)
# Reset Ctrl+C count when starting new message processing
signal_handler.reset_count()
# Process the message
if process_runner.process_message(message):
try:
# Set the current process for signal handling
signal_handler.set_process(process_runner.current_process)
result = process_runner.process_message(user_input)
print() # Add spacing for successful processing
else:
print_formatted_text(HTML('<red>Failed to process message</red>'))
except Exception as e:
print_formatted_text(HTML(f'<red>Failed to process message: {e}</red>'))
finally:
# Clear the process reference
signal_handler.set_process(None)
except KeyboardInterrupt:
# KeyboardInterrupt should be handled by the signal handler now
@ -269,8 +231,8 @@ def run_cli_entry(resume_conversation_id: str | None = None) -> None:
finally:
# Clean up resources
if process_runner:
process_runner.stop()
signal_handler.uninstall_handler()
process_runner.cleanup()
signal_handler.uninstall()
# Clean up terminal state
_restore_tty()

4
openhands-cli/openhands_cli/simple_main.py
View File

@ -18,7 +18,7 @@ from prompt_toolkit import print_formatted_text
from prompt_toolkit.formatted_text import HTML
from openhands_cli.argparsers.main_parser import create_main_parser
from openhands_cli.signal_handler import SignalHandler
from openhands_cli.simple_signal_handler import SimpleSignalHandler
def main() -> None:
@ -33,7 +33,7 @@ def main() -> None:
# Install basic signal handler for the main process
# The agent_chat module will install its own more sophisticated handler
signal_handler = SignalHandler()
signal_handler = SimpleSignalHandler()
try:
if args.command == 'serve':

124
openhands-cli/openhands_cli/simple_process_runner.py Normal file
View File

@ -0,0 +1,124 @@
"""
Simple process-based conversation runner for OpenHands CLI.
Only the actual conversation running (process_message) is wrapped in a separate process.
All other methods run in the main process.
"""
import multiprocessing
from typing import Any, Callable, Optional
from openhands.core.main import run_controller
from openhands.events.action import MessageAction
from openhands.events.observation import AgentFinishObservation
def _run_conversation_in_process(conversation_id: str, setup_func: Callable, message: str, result_queue: multiprocessing.Queue):
"""Run the conversation in a separate process."""
try:
# Set up the conversation components
controller, agent, runtime = setup_func()
# Create and process the message action
action = MessageAction(content=message)
# Run the controller with the action
result = run_controller(
controller=controller,
agent=agent,
runtime=runtime,
initial_user_action=action,
max_iterations=100,
max_budget_per_task=None,
agent_to_llm_config=None,
agent_configs=None,
)
# Put the result in the queue
result_queue.put(('success', result))
except KeyboardInterrupt:
result_queue.put(('interrupted', None))
except Exception as e:
result_queue.put(('error', str(e)))
class SimpleProcessRunner:
"""Simple conversation runner that only uses multiprocessing for the actual conversation."""
def __init__(self, conversation_id: str, setup_func: Callable):
self.conversation_id = conversation_id
self.setup_func = setup_func
self.current_process: Optional[multiprocessing.Process] = None
self.result_queue: Optional[multiprocessing.Queue] = None
# Set up conversation components in main process for non-process methods
self.controller, self.agent, self.runtime = setup_func()
def process_message(self, message: str) -> Any:
"""Process a message in a separate process."""
# Create queue for result
self.result_queue = multiprocessing.Queue()
# Create and start process
self.current_process = multiprocessing.Process(
target=_run_conversation_in_process,
args=(self.conversation_id, self.setup_func, message, self.result_queue)
)
self.current_process.start()
# Wait for result
try:
result_type, result_data = self.result_queue.get()
self.current_process.join()
if result_type == 'success':
return result_data
elif result_type == 'interrupted':
return AgentFinishObservation(content="Agent was interrupted by user")
else:
raise Exception(f"Process error: {result_data}")
except Exception as e:
if self.current_process.is_alive():
self.current_process.terminate()
self.current_process.join(timeout=2)
if self.current_process.is_alive():
self.current_process.kill()
self.current_process.join()
raise e
finally:
self.current_process = None
self.result_queue = None
def get_status(self) -> dict:
"""Get conversation status (runs in main process)."""
return {
'conversation_id': self.conversation_id,
'agent_state': getattr(self.agent, 'state', None),
'is_running': self.current_process is not None and self.current_process.is_alive()
}
def toggle_confirmation_mode(self) -> bool:
"""Toggle confirmation mode (runs in main process)."""
if hasattr(self.agent, 'confirmation_mode'):
self.agent.confirmation_mode = not getattr(self.agent, 'confirmation_mode', False)
return self.agent.confirmation_mode
return False
def resume(self) -> None:
"""Resume the agent (runs in main process)."""
if hasattr(self.agent, 'state'):
self.agent.state.resume()
def cleanup(self) -> None:
"""Clean up resources."""
if self.current_process and self.current_process.is_alive():
self.current_process.terminate()
self.current_process.join(timeout=2)
if self.current_process.is_alive():
self.current_process.kill()
self.current_process.join()
if hasattr(self.runtime, 'close'):
self.runtime.close()

67
openhands-cli/openhands_cli/simple_signal_handler.py Normal file
View File

@ -0,0 +1,67 @@
"""
Simple signal handling for Ctrl+C behavior in OpenHands CLI.
- First Ctrl+C: Attempt graceful pause of the agent
- Second Ctrl+C: Immediately kill the process
"""
import signal
import time
from typing import Optional
from prompt_toolkit import HTML, print_formatted_text
class SimpleSignalHandler:
"""Simple signal handler that tracks Ctrl+C presses and manages a subprocess."""
def __init__(self):
self.ctrl_c_count = 0
self.last_ctrl_c_time = 0.0
self.timeout = 3.0 # Reset counter after 3 seconds
self.original_handler = None
self.current_process: Optional[object] = None
def install(self) -> None:
"""Install the signal handler."""
self.original_handler = signal.signal(signal.SIGINT, self._handle_ctrl_c)
def uninstall(self) -> None:
"""Restore the original signal handler."""
if self.original_handler is not None:
signal.signal(signal.SIGINT, self.original_handler)
self.original_handler = None
def reset_count(self) -> None:
"""Reset the Ctrl+C count (called when starting new message processing)."""
self.ctrl_c_count = 0
self.last_ctrl_c_time = 0.0
def set_process(self, process) -> None:
"""Set the current process to manage."""
self.current_process = process
def _handle_ctrl_c(self, signum: int, frame) -> None:
"""Handle Ctrl+C signal."""
current_time = time.time()
# Reset counter if too much time has passed
if current_time - self.last_ctrl_c_time > self.timeout:
self.ctrl_c_count = 0
self.ctrl_c_count += 1
self.last_ctrl_c_time = current_time
if self.ctrl_c_count == 1:
print_formatted_text(HTML('<yellow>Received Ctrl+C. Attempting to pause agent...</yellow>'))
if self.current_process and self.current_process.is_alive():
self.current_process.terminate()
print_formatted_text(HTML('<yellow>Press Ctrl+C again within 3 seconds to force kill.</yellow>'))
else:
print_formatted_text(HTML('<yellow>No active process to pause.</yellow>'))
else:
print_formatted_text(HTML('<red>Received second Ctrl+C. Force killing process...</red>'))
if self.current_process and self.current_process.is_alive():
self.current_process.kill()
import os
os._exit(1)