bug #38493 [HttpClient] Fix CurlHttpClient memory leak (HypeMC)
This PR was squashed before being merged into the 4.4 branch.
Discussion
----------
[HttpClient] Fix CurlHttpClient memory leak
| Q | A
| ------------- | ---
| Branch? | 4.4
| Bug fix? | yes
| New feature? | no
| Deprecations? | no
| Tickets | Fix #38492
| License | MIT
| Doc PR | -
Commits
-------
aeb4ddf316
[HttpClient] Fix CurlHttpClient memory leak
This commit is contained in:
commit
1cef665f49
@ -17,7 +17,6 @@ use Symfony\Component\HttpClient\Chunk\InformationalChunk;
|
||||
use Symfony\Component\HttpClient\Exception\TransportException;
|
||||
use Symfony\Component\HttpClient\Internal\ClientState;
|
||||
use Symfony\Component\HttpClient\Internal\CurlClientState;
|
||||
use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface;
|
||||
use Symfony\Contracts\HttpClient\ResponseInterface;
|
||||
|
||||
/**
|
||||
@ -205,14 +204,9 @@ final class CurlResponse implements ResponseInterface
|
||||
return; // Unused pushed response
|
||||
}
|
||||
|
||||
$e = null;
|
||||
$this->doDestruct();
|
||||
} catch (HttpExceptionInterface $e) {
|
||||
throw $e;
|
||||
} finally {
|
||||
if ($e ?? false) {
|
||||
throw $e;
|
||||
}
|
||||
$multi = clone $this->multi;
|
||||
|
||||
$this->close();
|
||||
|
||||
@ -221,6 +215,8 @@ final class CurlResponse implements ResponseInterface
|
||||
$this->multi->dnsCache->evictions = $this->multi->dnsCache->evictions ?: $this->multi->dnsCache->removals;
|
||||
$this->multi->dnsCache->removals = $this->multi->dnsCache->hostnames = [];
|
||||
}
|
||||
|
||||
$this->multi = $multi;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -16,7 +16,6 @@ use Symfony\Component\HttpClient\Chunk\FirstChunk;
|
||||
use Symfony\Component\HttpClient\Exception\TransportException;
|
||||
use Symfony\Component\HttpClient\Internal\ClientState;
|
||||
use Symfony\Component\HttpClient\Internal\NativeClientState;
|
||||
use Symfony\Contracts\HttpClient\Exception\HttpExceptionInterface;
|
||||
use Symfony\Contracts\HttpClient\ResponseInterface;
|
||||
|
||||
/**
|
||||
@ -87,14 +86,9 @@ final class NativeResponse implements ResponseInterface
|
||||
public function __destruct()
|
||||
{
|
||||
try {
|
||||
$e = null;
|
||||
$this->doDestruct();
|
||||
} catch (HttpExceptionInterface $e) {
|
||||
throw $e;
|
||||
} finally {
|
||||
if ($e ?? false) {
|
||||
throw $e;
|
||||
}
|
||||
$multi = clone $this->multi;
|
||||
|
||||
$this->close();
|
||||
|
||||
@ -103,6 +97,8 @@ final class NativeResponse implements ResponseInterface
|
||||
$this->multi->responseCount = 0;
|
||||
$this->multi->dnsCache = [];
|
||||
}
|
||||
|
||||
$this->multi = $multi;
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -13,6 +13,8 @@ namespace Symfony\Component\HttpClient\Tests;
|
||||
|
||||
use Symfony\Component\HttpClient\Exception\ClientException;
|
||||
use Symfony\Component\HttpClient\Exception\TransportException;
|
||||
use Symfony\Component\HttpClient\Internal\ClientState;
|
||||
use Symfony\Contracts\HttpClient\Exception\RedirectionExceptionInterface;
|
||||
use Symfony\Contracts\HttpClient\Test\HttpClientTestCase as BaseHttpClientTestCase;
|
||||
|
||||
abstract class HttpClientTestCase extends BaseHttpClientTestCase
|
||||
@ -120,4 +122,26 @@ abstract class HttpClientTestCase extends BaseHttpClientTestCase
|
||||
throw $e;
|
||||
}
|
||||
}
|
||||
|
||||
public function testHandleIsRemovedOnException()
|
||||
{
|
||||
$client = $this->getHttpClient(__FUNCTION__);
|
||||
|
||||
try {
|
||||
$client->request('GET', 'http://localhost:8057/304');
|
||||
$this->fail(RedirectionExceptionInterface::class.' expected');
|
||||
} catch (RedirectionExceptionInterface $e) {
|
||||
// The response content-type mustn't be json as that calls getContent
|
||||
// @see src/Symfony/Component/HttpClient/Exception/HttpExceptionTrait.php:58
|
||||
$this->assertStringNotContainsString('json', $e->getResponse()->getHeaders(false)['content-type'][0] ?? '');
|
||||
|
||||
$r = new \ReflectionProperty($client, 'multi');
|
||||
$r->setAccessible(true);
|
||||
/** @var ClientState $clientState */
|
||||
$clientState = $r->getValue($client);
|
||||
|
||||
$this->assertCount(0, $clientState->handlesActivity);
|
||||
$this->assertCount(0, $clientState->openHandles);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -119,6 +119,10 @@ class MockHttpClientTest extends HttpClientTestCase
|
||||
$this->markTestSkipped("MockHttpClient doesn't timeout on destruct");
|
||||
break;
|
||||
|
||||
case 'testHandleIsRemovedOnException':
|
||||
$this->markTestSkipped("MockHttpClient doesn't cache handles");
|
||||
break;
|
||||
|
||||
case 'testGetRequest':
|
||||
array_unshift($headers, 'HTTP/1.1 200 OK');
|
||||
$responses[] = new MockResponse($body, ['response_headers' => $headers]);
|
||||
|
@ -157,7 +157,7 @@ switch ($vars['REQUEST_URI']) {
|
||||
exit;
|
||||
|
||||
case '/json':
|
||||
header("Content-Type: application/json");
|
||||
header('Content-Type: application/json');
|
||||
echo json_encode([
|
||||
'documents' => [
|
||||
['id' => '/json/1'],
|
||||
@ -170,7 +170,7 @@ switch ($vars['REQUEST_URI']) {
|
||||
case '/json/1':
|
||||
case '/json/2':
|
||||
case '/json/3':
|
||||
header("Content-Type: application/json");
|
||||
header('Content-Type: application/json');
|
||||
echo json_encode([
|
||||
'title' => $vars['REQUEST_URI'],
|
||||
]);
|
||||
|
Reference in New Issue
Block a user