Avoid FD (file descriptor) leak #560

Saga4 · 2025-07-18T20:29:43Z

Testing it with https://github.com/Saga4/core/pulls where kept getting error:

with Popen(*popenargs, kwargs) as process:
         ~~~~~^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniforge3/envs/core/lib/python3.13/subprocess.py", line 1005, in init**
    errread, errwrite) = self._get_handles(stdin, stdout, stderr)
                         ~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniforge3/envs/core/lib/python3.13/subprocess.py", line 1731, in gethandles
    c2pread, c2pwrite = os.pipe()
                        ~~~~~~~^^
OSError: [Errno 24] Too many open files
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
  File "/home/ubuntu/core/venv/bin/codeflash", line 8, in <module>
    sys.exit(main())
             ~~~~^^
  File "/home/ubuntu/core/venv/lib/python3.13/site-packages/codeflash/main.py", line 46, in main
    optimizer.run_with_args(args)
    ~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/home/ubuntu/core/venv/lib/python3.13/site-packages/codeflash/optimization/optimizer.py", line 390, in run_with_args
    optimizer.run()
    ~~~~~~~~~~~~~^^
  File "/home/ubuntu/core/venv/lib/python3.13/site-packages/codeflash/optimization/optimizer.py", line 349, in run
    self.cleanup_temporary_paths()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/home/ubuntu/core/venv/lib/python3.13/site-packages/codeflash/optimization/optimizer.py", line 381, in cleanup_temporary_paths
    get_run_tmp_file.tmpdir.cleanup()
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
  File "/home/ubuntu/miniforge3/envs/core/lib/python3.13/tempfile.py", line 954, in cleanup
    self._rmtree(self.name, ignore_errors=self._ignore_cleanup_errors)
    ~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniforge3/envs/core/lib/python3.13/tempfile.py", line 934, in _rmtree
    _shutil.rmtree(name, onexc=onexc)
    ~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniforge3/envs/core/lib/python3.13/shutil.py", line 763, in rmtree
    rmtreesafe_fd(stack, onexc)
    ~~~~~~~~~~~~~~~^^^^^^^^^^^^^^
  File "/home/ubuntu/miniforge3/envs/core/lib/python3.13/shutil.py", line 707, in rmtreesafe_fd
    onexc(func, path, err)
    ~~~~~^^^^^^^^^^^^^^^^^
  File "/home/ubuntu/miniforge3/envs/core/lib/python3.13/shutil.py", line 682, in rmtreesafe_fd
    with os.scandir(topfd) as scandir_it:
         ~~~~~~~~~~^^^^^^^
OSError: [Errno 24] Too many open files: '/tmp/codeflash__tgzk05c'

User description

Add precautions to FD leaks
Add precautions to FD leaks2

PR Type

Bug fix

Description

Close SQLite connections after usage
Use context manager for devnull file
Ensure subprocess pipes are closed
Add Makefile with leak test command

File Walkthrough

Relevant files

Error handling

replay_test.py `Add SQLite connection cleanup` codeflash/benchmarking/replay_test.py Wrap SQLite operations in try/finally Close DB connection in get_next_arg_and_return Ensure conn.close in generate_replay_test	+79/-68
beta.py `Manage devnull file context` codeflash/lsp/beta.py Use context manager for os.devnull writer Redirect stdout within 'with' block Remove manual open(os.devnull) call	+1/-2
codeflash_capture.py `Close capture database connection` codeflash/verification/codeflash_capture.py Move cursor creation inside try block Close SQLite connection in finally Balance gc.disable/enable calls properly	+47/-44
test_runner.py `Ensure subprocess pipes closed` codeflash/verification/test_runner.py Replace subprocess.run with Popen Explicitly close stdin/stdout/stderr pipes Handle timeouts with kill and terminate	+25/-1

Configuration changes

Makefile `Add development Makefile` Makefile Add comprehensive development Makefile Include test-file-handles leak test	+221/-0

github-actions · 2025-07-18T20:31:02Z

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review Resource Leak The DB connection in `get_next_arg_and_return` is closed in a `finally` block that only runs when the generator is fully consumed or garbage-collected. If iteration is stopped early, the connection remains open. try: cur = db.cursor() limit = num_to_get if class_name is not None: cursor = cur.execute( "SELECT * FROM benchmark_function_timings WHERE benchmark_function_name = ? AND function_name = ? AND file_path = ? AND class_name = ? LIMIT ?", (benchmark_function_name, function_name, file_path, class_name, limit), ) else: cursor = cur.execute( "SELECT * FROM benchmark_function_timings WHERE benchmark_function_name = ? AND function_name = ? AND file_path = ? AND class_name = '' LIMIT ?", (benchmark_function_name, function_name, file_path, limit), ) while (val := cursor.fetchone()) is not None: yield val[9], val[10] # pickled_args, pickled_kwargs finally: db.close() Exception Pickling Using `pickle.dumps(exception)` for arbitrary exception objects may fail if the exception isn't serializable. Consider serializing exception details instead of the raw object. exception = None gc.disable() try: counter = time.perf_counter_ns() wrapped(args, kwargs) codeflash_duration = time.perf_counter_ns() - counter except Exception as e: codeflash_duration = time.perf_counter_ns() - counter exception = e Install Command* The target uses `uv pip install .`, which may be incorrect. It probably should be a direct `pip install .` or use `uv run pip install .`. uv pip install .

github-actions · 2025-07-18T20:31:58Z

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Check args before accessing Add a guard to ensure there is at least one positional argument before accessing `args[0]`, so you avoid an IndexError when the wrapper is invoked without arguments. codeflash/verification/codeflash_capture.py [137-142] -if hasattr(args[0], "__dict__"): - instance_state = args[ - 0 - ].__dict__ # self is always the first argument, this is ensured during instrumentation -else: +if not args or not hasattr(args[0], "__dict__"): raise ValueError("Instance state could not be captured.") +instance_state = args[0].__dict__ Suggestion importance[1-10]: 5 __ Why: Adding a guard for an empty `args` tuple prevents a potential `IndexError` and safely raises a `ValueError`, improving the robustness of the wrapper.	Low

KRRT7 · 2025-07-21T18:34:18Z

codeflash/verification/test_runner.py

+        proc = subprocess.Popen(
+            cmd_list, stdout=subprocess.PIPE, stderr=subprocess.PIPE, stdin=subprocess.PIPE, cwd=cwd, env=env, text=True
+        )
+        try:
+            stdout, stderr = proc.communicate(timeout=timeout)
+            returncode = proc.returncode
+        except subprocess.TimeoutExpired:
+            proc.kill()
+            try:
+                stdout, stderr = proc.communicate(timeout=5)
+            except subprocess.TimeoutExpired:
+                proc.terminate()
+                stdout, stderr = "", "Process terminated due to timeout"
+            returncode = -1
+        finally:
+            # Ensure all pipes are closed
+            if proc.stdin:
+                proc.stdin.close()
+            if proc.stdout:
+                proc.stdout.close()
+            if proc.stderr:
+                proc.stderr.close()
+
+        return subprocess.CompletedProcess(cmd_list, returncode, stdout, stderr)


subprocess.run does this under the hood, doesn't make sense to reimplement it here.

https://github.com/python/cpython/blob/main/Lib/subprocess.py#L511

codeflash/verification/codeflash_capture.py

…manual Popen

KRRT7 · 2025-07-24T22:01:14Z

Makefile

we (Saurabh and I) don't really feel like a makefile is all that useful for us currently

Okay, as we are not having many contributors?
I added because of thinking about ongoing hackathon.

that's fair, but I don't think the hackathon participants will contribute to codeflash itself.

I think we can revisit something similar once uv adds their task runner which will be functionally equivalent

Okay makes sense. We can just close this PR as of now as I didnt see much improvements in FD leak while optimizing a large repo throwing FD leaks error with this.

Saga4 added 2 commits July 19, 2025 01:56

Add precautions to FD leaks

bd05e72

Add precautions to FD leaks2

a388ee9

github-actions bot added the Review effort 3/5 label Jul 18, 2025

Saga4 changed the title ~~saga4/FD leak~~ Avoid FD (file descriptor) leak Jul 18, 2025

Saga4 requested review from aseembits93, misrasaurabh1 and mohammedahmed18 July 18, 2025 20:41

KRRT7 requested changes Jul 21, 2025

View reviewed changes

KRRT7 and others added 2 commits July 24, 2025 13:44

Merge branch 'main' into saga4/FD_leak

2a0682a

Revert subprocess changes in file.py - use subprocess.run instead of …

1b3c83c

…manual Popen

Saga4 requested a review from KRRT7 July 24, 2025 21:40

Merge branch 'main' into saga4/FD_leak

b598916

KRRT7 requested changes Jul 24, 2025

View reviewed changes

Merge branch 'main' into saga4/FD_leak

708d714

Saga4 closed this Jul 24, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Avoid FD (file descriptor) leak #560

Avoid FD (file descriptor) leak #560

Uh oh!

Saga4 commented Jul 18, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Jul 18, 2025

Uh oh!

github-actions bot commented Jul 18, 2025

Uh oh!

KRRT7 Jul 21, 2025 •

edited

Loading

Uh oh!

Saga4 Jul 24, 2025

Uh oh!

Uh oh!

KRRT7 Jul 24, 2025

Uh oh!

Saga4 Jul 24, 2025

Uh oh!

KRRT7 Jul 24, 2025

Uh oh!

Saga4 Jul 24, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Avoid FD (file descriptor) leak #560

Avoid FD (file descriptor) leak #560

Uh oh!

Conversation

Saga4 commented Jul 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

User description

PR Type

Description

File Walkthrough

Uh oh!

github-actions bot commented Jul 18, 2025

PR Reviewer Guide 🔍

Uh oh!

github-actions bot commented Jul 18, 2025

PR Code Suggestions ✨

Uh oh!

KRRT7 Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Saga4 Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

KRRT7 Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

Saga4 Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

KRRT7 Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

Saga4 Jul 24, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Saga4 commented Jul 18, 2025 •

edited

Loading

KRRT7 Jul 21, 2025 •

edited

Loading