fix(agbench): close console_log.txt file handle in run_scenario_in_docker#7831
fix(agbench): close console_log.txt file handle in run_scenario_in_docker#7831Ricardo-M-L wants to merge 1 commit into
Conversation
…cker run_scenario_in_docker opened console_log.txt with a bare open() and never closed it. Since this function is invoked once per scenario/repetition during a benchmark run, the leaked write handles accumulate across the run and can exhaust the process file-descriptor limit on large suites (and on Windows the unclosed handle can keep the file locked). Wrap the streaming/log-writing block in a with statement so the handle is always released on the normal path, on StopIteration, and when exiting via sys.exit(1) after a keyboard interrupt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@Ricardo-M-L please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
avinashkamat48
left a comment
There was a problem hiding this comment.
The file handle is now closed by the context manager, but the PR does not add a regression test around the timeout/KeyboardInterrupt paths that write to console_log.txt after the log loop exits. Those are the paths most likely to regress with this indentation change. Could you add a small unit test or mocked container case proving the timeout footer is written and the file is closed after run_scenario_in_docker exits?
|
Hi @ekzhu @jackgerrits! This PR fixes a file handle leak in Please let me know if you'd like me to add tests, adjust the approach, or rebase onto the latest main. Thanks! |
Why
run_scenario_in_docker(inagbench/run_cmd.py) opensconsole_log.txtwith a bareopen()and never closes it:This function is called once per scenario and per repetition during a benchmark run (
run_scenario_in_dockeris invoked inside the per-instance / per-repetition loop). The leaked write handles therefore accumulate over the course of a run. On large benchmark suites this can exhaust the process file-descriptor limit, and on Windows the unclosed handle keepsconsole_log.txtlocked.Note the sibling handle in the same module (
file_handleinrun_scenario) is closed explicitly — this one was simply missed.Fix
Wrap the streaming / log-writing block in a
withstatement so the handle is always released:StopIterationpath,sys.exit(1)after aKeyboardInterrupt(thefinallyfrom the context manager runs before the process exits).No behavior changes other than guaranteed handle release. The
if exiting: sys.exit(1)check is intentionally kept outside thewithblock so the file is flushed and closed before the process exits.Verification
python -m py_compilepasses.if stopping:epilogue) is now nested under thewith open(...) as log_file:context manager.