php_cmd() allocates the undefined-value string before it retries, so every retry that succeeds loses the allocation.
php.c:90 (develop):
retry:
bytes = write(php_processes[php_process].php_write_fd, command, strlen(command));
if (bytes <= 0) {
result_string = strdup("U"); /* allocated here */
...
php_close(php_process);
php_init(php_process);
retries++;
if (retries < 3) {
goto retry; /* ...and lost here if the retry works */
}
} else {
result_string = php_readpipe(php_process, command); /* overwrites it */
...
}
The strdup("U") is the value to report if the script server cannot be reached. But the code allocates it, restarts the server, and jumps back to the write. If the replacement server answers, result_string is reassigned from php_readpipe() and the earlier allocation is unreachable.
The leak is small, two bytes plus allocator overhead per occurrence, but it fires on exactly the path this code exists to handle: a script server that has died and is being replaced. A poller whose script servers flap repeats it once per item per restart.
Confirmed with LeakSanitizer against a test that lets the restart succeed:
Direct leak of 4 byte(s) in 2 object(s) allocated from:
#0 strdup
#1 php_cmd /src/php.c:119
Suggested fix is to allocate only after the retries are spent, which leaves the give-up behaviour identical:
if (bytes <= 0) {
SPINE_LOG(...);
php_close(php_process);
php_init(php_process);
retries++;
if (retries < 3) {
goto retry;
}
result_string = strdup("U");
} else {
...
}
Present on develop and 1.2.x. Fix and a test are in #597.
php_cmd()allocates the undefined-value string before it retries, so every retry that succeeds loses the allocation.php.c:90(develop):The
strdup("U")is the value to report if the script server cannot be reached. But the code allocates it, restarts the server, and jumps back to the write. If the replacement server answers,result_stringis reassigned fromphp_readpipe()and the earlier allocation is unreachable.The leak is small, two bytes plus allocator overhead per occurrence, but it fires on exactly the path this code exists to handle: a script server that has died and is being replaced. A poller whose script servers flap repeats it once per item per restart.
Confirmed with LeakSanitizer against a test that lets the restart succeed:
Suggested fix is to allocate only after the retries are spent, which leaves the give-up behaviour identical:
Present on
developand1.2.x. Fix and a test are in #597.