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

Memory leak #95

Closed
patrickcarlohickman opened this issue Oct 31, 2022 · 1 comment · Fixed by #96
Closed

Memory leak #95

patrickcarlohickman opened this issue Oct 31, 2022 · 1 comment · Fixed by #96
Assignees

Comments

@patrickcarlohickman
Copy link

This issue expands on issue #65, which noted a memory issue. This is an explanation for the memory leak and why the suggested unset() won't fix it. Additionally, the original poster's solution of manually calling close() won't prevent the memory leak in PHP8+.

Minimum reproducible code:

$curl = new Curl\Curl();
unset($curl); // destructor not called; object still in memory

In short, the destructor is never being called. Even if you manually unset() the object, the destructor will not be called. This means the object is never garbage collected, the curl resource is never closed, and the memory is never cleaned up, until the end of the script.

From the PHP manual:

The destructor method will be called as soon as there are no other references to a particular object...

The reason the destructor is not called is because there is a circular reference between the Curl object and the underlying curl resource. The Curl constructor calls the init() function, and the init() function has this line:

$this->setOpt(CURLOPT_HEADERFUNCTION, array($this, 'addResponseHeaderLine'));

The Curl object has a reference to the curl resource, and now, because of that line, the curl resource has a reference to the Curl object.

Therefore, when you call unset($curl) in the code above, the variable will be unset, but the object will not actually be destroyed because the curl resource still has a reference to it.

This is why manually calling the close() method removes the memory leak (at least before PHP8). When the close() method is called, it closes the curl resource, which removes the reference to the Curl object, leaving the only reference being the one we created. Therefore, when we unset our reference (or our reference goes out of scope), there will be no more references and the object can be destroyed.

However, this is not true in PHP8. From the PHP8 migration guide:

curl_init() will now return a CurlHandle object rather than a resource. The curl_close() function no longer has an effect, instead the CurlHandle instance is automatically destroyed if it is no longer referenced.

In PHP8, the curl functions were changed to operate on a CurlHandle object instead of a curl resource. Additionally, the curl_close() function was updated to have no effect. Therefore, in PHP8, manually calling the close() method will not resolve the memory leak because it doesn't actually do anything. The circular reference between the Curl object and the CurlHandle object will still exist.

The easiest solution would be to remove the line from the init() function and add it into the exec() function, so you can set it before the curl request and then unset it after the curl request:

$this->setOpt(CURLOPT_HEADERFUNCTION, array($this, 'addResponseHeaderLine'));
$this->response = curl_exec($this->curl);
$this->setOpt(CURLOPT_HEADERFUNCTION, null);

Thanks,
Patrick

This was referenced Oct 31, 2022
@nadar
Copy link
Member

nadar commented Nov 2, 2022

Hi @patrickcarlohickman

Thanks for this brilliant explanation and deep dive of that problem. Great work! Please see #96

@nadar nadar closed this as completed in #96 Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants