-
Notifications
You must be signed in to change notification settings - Fork 67
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
Memory leak #179
Comments
Thank you @dmarkic for bringing this up 👍 This is definitely an interesting one. We're currently not aware of any memory leaks in this project, but your input above seems like you may have found one. In order to make sure this is actually a memory leak, we need some additional tests for the event listeners (if they get removed successfully) to support this thesis. Is this something you can look into? |
Hello, Sorry for late reply, just came back from vacation. You mean tests with this |
This problem still exist. I changed private function onError(\Exception $error)
{
$this->rsState = self::RS_STATE_HEADER;
$this->resultFields = [];
// reject current command with error if we're currently executing any commands
// ignore unsolicited server error in case we're not executing any commands (connection will be dropped)
if ($this->currCommand !== null) {
$command = $this->currCommand;
$this->currCommand = null;
$command->emit('error', [$error]);
$command->removeAllListeners();
}
}
protected function onResultDone()
{
$command = $this->currCommand;
$this->currCommand = null;
$command->resultFields = $this->resultFields;
$command->emit('end');
$command->removeAllListeners();
$this->rsState = self::RS_STATE_HEADER;
$this->resultFields = [];
}
protected function onSuccess()
{
$command = $this->currCommand;
$this->currCommand = null;
if ($command instanceof QueryCommand) {
$command->affectedRows = $this->affectedRows;
$command->insertId = $this->insertId;
$command->warningCount = $this->warningCount;
$command->message = $this->message;
}
$command->emit('success');
$command->removeAllListeners();
}
public function onClose()
{
if ($this->currCommand !== null) {
$command = $this->currCommand;
$this->currCommand = null;
if ($command instanceof QuitCommand) {
$command->emit('success');
} else {
$command->emit('error', [new \RuntimeException(
'Connection closing (ECONNABORTED)',
\defined('SOCKET_ECONNABORTED') ? \SOCKET_ECONNABORTED : 103
)]);
}
$command->removeAllListeners();
}
} Try with that: $loop->addPeriodicTimer(
3,
function () use ($logger) {
//gc_collect_cycles();
$logger->info(">> Memory usage: " . number_format(memory_get_usage() / 1024 / 1024, 4, '.', '') . " MB");
}
);
...
for ($i = 0; $i < 10; $i++) {
$this->loop->addPeriodicTimer(
0.1,
function () {
// TODO This is causing memory leak. Solve this.
$this->pool
->query('SELECT \'' . str_repeat('A', 102400) . '\'')
->then(function (QueryResult $queryResult) {
//$this->logger->info(">> Query done");
})
->catch(function (Throwable $throwable) {
$this->logger->error(">> Query error: " . $throwable->getMessage());
});
}
);
} |
Probably I found the problem. In Edit: It seems this problem occurs in PHP8.2 only. In PHP8.1 it doesn't increasing memory usage.
As you can see memory is wandering between 92MB and 28MB. Garbage collector is cleaning it up. It is fine but if you handle PHP8.2 issues than the life will be better I think :) Thanks. |
@dmarkic We need a test that expects all listeners to be removed at the end, which would fail if this isn't currently the case. Depending on the outcome of this test we would then have the proof (or not) if this is actually the case here. @kodmanyagha Thanks for your input on this! This has already been fixed in #170 and was a oversight from #161. As I described in #175 (comment), we're currently working on a MySQL connection pool feature and plan this for the future We currently don't have the confirmation that there's actually a memory leak in this ticket, so I will close this for now. We can reopen this if we have a test to confirm the leak and then start working on a fix. |
Hello,
When command is finished, it stays in memory as
evenement/evenement
events are still registered with command and so command is never destructed.Version: 5.7.x
PHP Version: 7.4.33
A simple reproduction script:
Example output:
I've made few changes to
Io\Parser.php
to call$command->removeAllListeners()
in methods:It has solved the memory issue.
I think that's a major issue, should I create a simple PR where removeAllListeners() is called on command when it's finished?
Kind regards,
Dejan Markic
The text was updated successfully, but these errors were encountered: