fix: Investigate and fix memory leak in file editor by xingyaoww · Pull Request #60 · OpenHands/openhands-aci

xingyaoww · 2025-01-27T21:16:46Z

This PR fixes a memory leak issue in the file editor functionality by implementing a disk-based history manager.

Issue Description

When performing multiple file edit operations, memory usage grows significantly and consistently. A test was added that demonstrated:

Memory grew by approximately 19MB per 100 edits
After just 100 edits, memory usage increased by 71.6%
The growth was consistent and not temporary

Solution

Implemented several improvements to reduce memory usage:

Added FileHistoryManager class to manage file edit history:
- Uses disk-based storage instead of in-memory storage
- Maintains a fixed window of history entries per file (default 10)
- Implements a memory cache with size limits (5 files)
- Automatically cleans up temporary files
Modified OHEditor to use the new history manager:
- Removed in-memory history dictionary
- Updated all history operations to use disk storage
- Added proper cleanup on exit
Updated memory leak test:
- Added comprehensive memory monitoring
- Test now passes with stable memory usage

Test Results

The memory leak test now passes successfully, showing:

Memory usage remains stable
History entries are properly maintained
No memory leaks detected

Additional Notes

The solution maintains a constant memory footprint regardless of edit count
Old history entries are automatically removed when limits are reached
Disk storage is cleaned up properly when the editor is closed

This commit adds a comprehensive test to demonstrate and measure memory growth during file editing operations. The test: 1. Sets a memory limit of 256MB 2. Creates large file content to make memory issues more apparent 3. Performs multiple edits while monitoring memory usage 4. Tracks memory growth rate and fails if consistent growth is detected 5. Provides detailed memory statistics throughout the test run The test successfully demonstrates a memory leak where memory grows by approximately 19MB per 100 edits.

This commit implements several improvements to reduce memory usage in the file editor: 1. Added FileHistoryManager class to manage file edit history: - Uses disk-based storage instead of in-memory storage - Maintains a fixed window of history entries per file - Implements a memory cache with size limits - Automatically cleans up temporary files 2. Modified OHEditor to use the new history manager: - Removed in-memory history dictionary - Updated all history operations to use disk storage - Added proper cleanup on exit 3. Updated memory leak test: - Reduced test size to be more manageable - Added history size monitoring - Test now passes with stable memory usage The changes ensure that memory usage remains constant regardless of the number of edits performed.

- Add line-by-line reading for large files - Use streaming operations for file modifications - Add psutil to test dependencies - Update workflow to install test deps

- Keep only memory-efficient file reading in editor.py - Optimize history storage to avoid saving entire history at once

- Add size limit check (10MB max) - Add binary file detection - Add MIME type checking for text files - Add FileValidationError exception - Fix error message handling in base ToolError class - Add comprehensive tests for file validation

- Reduce file size to 9.5MB to stay under validation limit - Add garbage collection before memory measurement - Add better error handling and debug output - Add file size and line count verification - Improve memory usage reporting

enyst · 2025-01-31T10:10:03Z

The code LGTM. Interesting!

neubig

Looks great to me! I basically approve but just had a few small comments.

neubig · 2025-01-31T11:12:56Z

-from collections import defaultdict
 from pathlib import Path
-from typing import Literal, get_args
+from typing import Literal, Optional, get_args


| None probably works instead of Optional.

neubig · 2025-01-31T11:18:14Z

+
+        # Check file size (10MB limit)
+        file_size = os.path.getsize(path)
+        max_size = 10 * 1024 * 1024  # 10MB in bytes


Make this configurable?

Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>

- Replace Optional with | None - Make file size limit configurable

xingyaoww · 2025-01-31T15:00:39Z

Thanks @neubig ! Just got OpenHands revert an unintended change when it was addressing your comment :D

Will merge now

openhands-agent added 11 commits January 27, 2025 20:20

Fix memory leak in file operations:

cec5db8

- Add line-by-line reading for large files - Use streaming operations for file modifications - Add psutil to test dependencies - Update workflow to install test deps

Fix linting issues and add test for file read memory usage

7c9bf02

Update poetry.lock with psutil dependency

a6480b3

Fix str_replace line matching and test assertions

199b003

Fix test_file_read_memory_usage and improve str_replace

6578814

Fix linting issues

56fb395

Fix cache eviction to use LRU (Least Recently Used) strategy

df70e95

Simplify history management using diskcache

b137b66

Simplify changes:

7aa1e23

- Keep only memory-efficient file reading in editor.py - Optimize history storage to avoid saving entire history at once

xingyaoww force-pushed the test-memory-leak-file-edits branch from 3a2cc2c to 7aa1e23 Compare January 29, 2025 20:54

openhands-agent added 16 commits January 29, 2025 22:00

Fix test_file_read_memory_usage test

d39def8

- Reduce file size to 9.5MB to stay under validation limit - Add garbage collection before memory measurement - Add better error handling and debug output - Add file size and line count verification - Improve memory usage reporting

Fix linting issues: trailing whitespace, EOF, and code formatting

feb7007

Remove unused for_edit parameter from validate_file method

d907256

Replace manual string search with re.finditer for better reliability

ab6f5e3

Add safe line counting and consistent file validation

d2cb765

Add comprehensive tests for file editor error handling and validation

3f615b0

Add tab expansion tests and reorganize tests into separate files

80a7867

Fix linting issues in test files

26e151f

Fix code formatting in test_file_editor.py

61f7e05

Remove duplicate test file

cdc2ad6

Fix history key generation to use monotonic counter

e87f56b

Move unique tests from test_file_editor.py to test_basic_operations.py

133a77b

Remove duplicate test file

04aa50c

Bump version to 0.2.0 for memory leak fixes and improved file validation

26a40b4

Fix code formatting in test_history.py

e76fd50

xingyaoww marked this pull request as ready for review January 30, 2025 17:54

Add peak memory usage tests

9c3b69c

Fix memory tests to avoid interfering with pytest

f9e3ede

xingyaoww mentioned this pull request Jan 31, 2025

Update ACI to 0.2.0 to fix Memory Leak OpenHands/OpenHands#6559

Merged

1 task

enyst reviewed Jan 31, 2025

View reviewed changes

Comment thread openhands_aci/editor/history.py Outdated

neubig reviewed Jan 31, 2025

View reviewed changes

xingyaoww and others added 4 commits January 31, 2025 22:26

Update openhands_aci/editor/history.py

b9bb389

Co-authored-by: Engel Nyst <enyst@users.noreply.github.com>

Address PR comments:

43abbca

- Replace Optional with | None - Make file size limit configurable

Fix remaining Optional type hints and formatting

234971b

Optimize memory usage in insert operation

fb751e3

neubig approved these changes Jan 31, 2025

View reviewed changes

openhands-agent added 2 commits January 31, 2025 14:57

Fix memory usage in insert operation by using snippet for result

342478e

Revert back to using read_file for getting new content

cd9b9d0

xingyaoww merged commit ecd4439 into main Jan 31, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: Investigate and fix memory leak in file editor#60

fix: Investigate and fix memory leak in file editor#60
xingyaoww merged 35 commits into
mainfrom
test-memory-leak-file-edits

xingyaoww commented Jan 27, 2025

Uh oh!

Uh oh!

enyst commented Jan 31, 2025

Uh oh!

neubig left a comment

Uh oh!

neubig Jan 31, 2025

Uh oh!

Uh oh!

neubig Jan 31, 2025

Uh oh!

xingyaoww commented Jan 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

xingyaoww commented Jan 27, 2025

Issue Description

Solution

Test Results

Additional Notes

Uh oh!

Uh oh!

enyst commented Jan 31, 2025

Uh oh!

neubig left a comment

Choose a reason for hiding this comment

Uh oh!

neubig Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

neubig Jan 31, 2025

Choose a reason for hiding this comment

Uh oh!

xingyaoww commented Jan 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants