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

Query Logs are recorded in a static array, causes memory leaks #392

Closed
nazmulpcc opened this issue Oct 3, 2021 · 3 comments
Closed

Query Logs are recorded in a static array, causes memory leaks #392

nazmulpcc opened this issue Oct 3, 2021 · 3 comments
Labels

Comments

@nazmulpcc
Copy link
Contributor

nazmulpcc commented Oct 3, 2021

  • Octane Version: 1.0.12
  • Laravel Version: 8.62.0
  • PHP Version: 8.0.11
  • Server & Version: Swoole 4.6.7
  • Database Driver & Version: sqlite/mysql

Description:

I was using DB::enableQueryLog() and DB::getQueryLog() to record queries in a request and later process them(automatically find duplicate queries and others). But when I tried to integrate octane, I found that the queries are kept in a array between octane requests. Octane should clear this array after each requests to prevent memory leaks.

Reason

The array is defined in Illuminate/Database/Connection.php#L148, as the Database connection is reused between octane requests, the array on the object is never cleared.

Possible Solution

Maybe octane can call DB::flushQueryLog() after each request? [ For simplicity I am assuming only one connection is used ]

Steps To Reproduce:

  • Clone this repo
  • Run composer install && cp .env.example .env && touch database/database.sqlite && php artisan migrate. The project is set up to use sqlite as DB driver by default for testing convenience.
  • Run php artisan octane:start to start the server and then visit the homepage. After each refresh, you will see that the queries from all previous requests are also present. Also, the property database_connection_object_id in the response shows the current database connection object id which remains same.
@driesvints
Copy link
Member

@nazmulpcc Thanks. Your solution sounds reasonable. Can you send in a PR?

@nazmulpcc
Copy link
Contributor Author

Sure, I will send in a PR soon.

@driesvints
Copy link
Member

Thanks @nazmulpcc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants