Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

withFile memory usage #16701

Open
ludeus opened this issue Aug 23, 2022 · 3 comments
Open

withFile memory usage #16701

ludeus opened this issue Aug 23, 2022 · 3 comments
Milestone

Comments

@ludeus
Copy link
Contributor

ludeus commented Aug 23, 2022

Description

Downloading large files causes RAM issues.
Indeed, the file is sent by consecutive "echo" in "emitBody" but if we are in a buffer level, the file is stored in RAM.

I think there is a bug in the call of "flush()", here is its call in the method "ResponseEmiter::emit()"

        if (!headers_sent()) {
            $this->emitStatusLine($response);
            $this->emitHeaders($response);
        }
        $this->flush();

        $range = $this->parseContentRange($response->getHeaderLine('Content-Range'));
        if (is_array($range)) {
            $this->emitBodyRange($range, $response);
        } else {
            $this->emitBody($response);
        }

Looks like we want to flush the entire existing buffer before running "emitBody()"

But, the flush function is called with null param :

    protected function flush(?int $maxBufferLevel = null): void
    {
        if ($maxBufferLevel === null) {
            $maxBufferLevel = ob_get_level();
        }

        while (ob_get_level() > $maxBufferLevel) {
            ob_end_flush();
        }
    }

with a null "$maxBuffer Level", we don't go through the loop, so we might as well do nothing

CakePHP Version

4.4.4

PHP Version

No response

@ludeus ludeus added the defect label Aug 23, 2022
@markstory
Copy link
Member

with a null "$maxBuffer Level", we don't go through the loop, so we might as well do nothing

That's not entirely true. The application could have buffered output somewhere else and the current buffering level would be inferred.

In your scenario I'm assuming you have are starting an output buffer somewhere else or have automatic output buffering enabled?

@markstory markstory added this to the 4.4.5 milestone Aug 23, 2022
@markstory markstory added the http label Aug 23, 2022
@ndm2
Copy link
Contributor

ndm2 commented Aug 23, 2022

I think what he means is that this is essentially a no-op if no value is being provided explicitly, as ob_get_level() will never be greater than ob_get_level().

If the buffer was meant to be completely flushed and disabled by default, then $maxBufferLevel would need to be set to 0.

@markstory
Copy link
Member

Fair enough, I don't think that is contributing to additional memory usage though.

@markstory markstory modified the milestones: 4.4.5, 4.4.6 Aug 29, 2022
@markstory markstory modified the milestones: 4.4.6, 4.4.7 Oct 2, 2022
@markstory markstory modified the milestones: 4.4.7, 4.4.8 Oct 30, 2022
@markstory markstory modified the milestones: 4.4.8, 4.4.9 Dec 2, 2022
@markstory markstory modified the milestones: 4.4.9, 4.4.10 Dec 30, 2022
@othercorey othercorey modified the milestones: 4.4.10, 4.4.11 Jan 18, 2023
@markstory markstory modified the milestones: 4.4.11, 4.4.12 Feb 11, 2023
@markstory markstory modified the milestones: 4.4.12, 4.4.13 Mar 24, 2023
@markstory markstory modified the milestones: 4.4.13, 4.4.14 Apr 23, 2023
@markstory markstory modified the milestones: 4.4.14, 4.4.15 May 23, 2023
@markstory markstory modified the milestones: 4.4.15, 4.4.16 Jul 2, 2023
@markstory markstory modified the milestones: 4.4.16, 4.4.17 Aug 8, 2023
@markstory markstory modified the milestones: 4.4.17, 4.4.18 Aug 20, 2023
@markstory markstory modified the milestones: 4.4.18, 4.4.19 Sep 28, 2023
@markstory markstory modified the milestones: 4.4.19, 4.5.1 Oct 15, 2023
@dereuromark dereuromark added this to the 4.5.4 milestone Jan 24, 2024
@markstory markstory modified the milestones: 4.5.4, 4.5.5 Mar 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants