fix: prevent memory leak and fix real-time progress parsing for carriage returns#7
Conversation
…nd fix progress updates
Rclone often uses carriage returns (`\r`) for in-place progress updates in the terminal. The `ProgressParser` was previously splitting the output buffer strictly by newlines (`\n`) using `explode`. This caused it to accumulate a massive string in `lineBuffer` without ever processing the progress updates or clearing the memory, resulting in memory leaks and broken real-time progress parsing.
Changing `explode("\n", ...)` to `preg_split("/\r\n|\n|\r/", ...)` fixes the issue by supporting all line-ending formats correctly.
Co-authored-by: insign <1113045+insign@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
1 participant
This PR implements exactly one real improvement for the project: Fixing a memory leak and broken progress updates in
ProgressParser.The
ProgressParserwas usingexplode("\n", $this->lineBuffer)to process the output fromrclone. Sincerclone(and many other CLI tools) uses a carriage return (\r) to update progress on the same line in the terminal without outputting a full line feed (\n), the buffer kept growing indefinitely. This caused:I replaced the
explode("\n")withpreg_split("/\r\n|\n|\r/"). This allows the parser to split the incoming text correctly regardless of whether the line separator is a Windows CRLF (\r\n), a Unix LF (\n), or an old Mac / terminal in-place update CR (\r).Scope and Tests:
src/ProgressParser.phpwas touched../vendor/bin/phpunit --testsuite offline_no_docker)..mdfiles needed to be updated because the external API and documented behavior haven't changed.PR created automatically by Jules for task 17559374959749521812 started by @insign