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

Prevent Memory Leak when using hideRequestParameters and hideRequestHeaders #1350

Merged
merged 3 commits into from
May 29, 2023

Conversation

danilopinotti
Copy link
Contributor

This PR aims to prevent a memory leak when we call the method hideRequestHeaders or/and hideRequestParameters multiple times with the same parameters. Ex:

Telescope::hideRequestHeaders(['cookie', 'x-csrf-token']);
Telescope::hideRequestHeaders(['cookie', 'x-csrf-token']);

It seems like an inoffensive issue, but it causes a memory leak in tests:

class MemoryLeakTest extends TestCaseUnit
{
    public function test_leaks_memory_on_1000_iterations()
    {
        $this->app->flush();
        $this->app = null;

        for ($i = 1; $i < 500; ++$i) {
            $this->createApplication()
                ->flush();

            if (! ($i % 10)) {
                dump('Using ' . ((int) (memory_get_usage(true) / (1024 * 1024))) . 'MB as ' . $i . ' iterations.');
            }
        }

        $this->app = $this->createApplication();
    }
}

The code above was copied from https://darkghosthunter.medium.com/laravel-fixing-memory-leaks-on-tests-4fe87b87f0ad

@driesvints
Copy link
Member

This is definitely good but just fyi: Telescope shouldn't be enabled in tests.

@danilopinotti
Copy link
Contributor Author

danilopinotti commented May 29, 2023

This is definitely good but just fyi: Telescope shouldn't be enabled in tests.

I agree with it, but even with the option TELESCOPE_ENABLED=false, the provider is initialized (except when it be configured as this session: https://laravel.com/docs/10.x/telescope#local-only-installation)

@taylorotwell taylorotwell merged commit 12cf59c into laravel:4.x May 29, 2023
@danilopinotti danilopinotti deleted the patch-1 branch May 29, 2023 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants