Skip to content

Commit

Permalink
Use weak references to avoid the test causing a leak
Browse files Browse the repository at this point in the history
  • Loading branch information
schlessera committed Sep 11, 2023
1 parent dd91131 commit 61b3b14
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 12 deletions.
6 changes: 0 additions & 6 deletions src/Transport/Curl.php
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,6 @@ public function request($url, $headers = [], $data = [], $options = []) {
}
}

$this->hooks = $options['hooks'];

$this->setup_handle($url, $headers, $data, $options);

$options['hooks']->dispatch('curl.before_send', [&$this->handle]);

$this->response_data = '';
$this->response_bytes = 0;
$this->response_byte_limit = false;
Expand Down
22 changes: 16 additions & 6 deletions tests/Transport/Curl/CurlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
namespace WpOrg\Requests\Tests\Transport\Curl;

use CurlHandle;
use WeakReference;
use WpOrg\Requests\Exception;
use WpOrg\Requests\Hooks;
use WpOrg\Requests\Requests;
Expand All @@ -15,7 +16,10 @@ final class CurlTest extends BaseTestCase {
/**
* Temporary storage of the cURL handle to assert against.
*
* @var null|resource|\CurlHandle
* The handle is stored as a weak reference in order to avoid the test itself
* becoming the source of the memory leak due to locking the resource.
*
* @var null|WeakReference
*/
protected $curl_handle;

Expand All @@ -34,14 +38,20 @@ protected function getOptions($other = []) {

$this->curl_handle = null;

// On PHP < 7.2, we lack the capability to store weak references.
// In this case, we just skip the memory leak testing.
if (version_compare(PHP_VERSION, '7.2.0') < 0) {
return $options;
}

if (!array_key_exists('hooks', $options)) {
$options['hooks'] = new Hooks();
}

$options['hooks']->register(
'curl.before_request',
function ($handle) {
$this->curl_handle = $handle;
$this->curl_handle = WeakReference::create($handle);
}
);

Expand All @@ -54,19 +64,19 @@ function ($handle) {
* This is used for asserting that cURL handles are not leaking memory.
*/
protected function assert_post_conditions() {
if ($this->curl_handle === null) {
if (version_compare(PHP_VERSION, '7.2.0') < 0 || !$this->curl_handle instanceof WeakReference) {
// No cURL handle was used during this particular test scenario.
return;
}

if ($this->curl_handle instanceof CurlHandle) {
if ($this->curl_handle->get() instanceof CurlHandle) {
// CURL handles have been changed from resources into CurlHandle
// objects starting with PHP 8.0, which don;t need to be closed.
return;
}

if ($this->shouldClosedResourceAssertionBeSkipped($this->curl_handle) === false) {
$this->assertIsClosedResource($this->curl_handle);
if ($this->shouldClosedResourceAssertionBeSkipped($this->curl_handle->get()) === false) {
$this->assertIsClosedResource($this->curl_handle->get());
}
}

Expand Down

0 comments on commit 61b3b14

Please sign in to comment.