[HttpClient] Fix Failed to open stream: Too many open files #45073

adrienfr · 2022-01-19T09:38:09Z

Q	A
Branch?	4.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets	Fix #44900
License	MIT

With a large PHPUnit tests suite, I receive this error "Failed to open stream: Too many open files".
I found the original code from @nicolas-grekas here and with small adjustments, it resolves my issue.

Warning: tests for HttpClient are OK but would someone try with a real HTTP/2 client push? I was not able to test it.

src/Symfony/Component/HttpClient/Internal/CurlClientState.php

adrienfr · 2022-01-19T10:51:18Z

@nicolas-grekas I think the issue is related to $this->handle.
If I keep the static way and create this :

$multi = (object) [
      //'handle' => $this->handle,
      //'handle' => &$this->handle,
      'logger' => &$this->logger,
      'pushedResponses' => &$this->pushedResponses,
      'dnsCache' => &$this->dnsCache,
      'handlesActivity' => &$this->handlesActivity,
      'openHandles' => &$this->openHandles,
      'execCounter' => &$this->execCounter,
      'pauseExpiries' => &$this->pauseExpiries,
      'lastTimeout' => &$this->lastTimeout,
  ];

  // Keep a dummy "onPush" reference to work around a refcount bug in PHP
  curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, $this->onPush = static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes) {
      return self::handlePush($parent, $pushed, $requestHeaders, $multi, $maxPendingPushes);
  });

It works. But I soon as I uncomment one of the handle key in my $multi object, I have the "failed to open stream: Too many open files" error, as if this $this->handle is not properly "reset".

One other thing I noticed, a lot more CurlMultiHandle usage in newer version:

SF 5.3.7 :
CurlHttpClient just before curl_multi_setopt($this->multi->handle, \CURLMOPT_PUSHFUNCTION, ...

var_dump($this->multi->handle);

=>

.  854 / 4775 ( 17%)
resource(15232) of type (curl_multi)
.resource(15342) of type (curl_multi)
.resource(15383) of type (curl_multi)

SF 5.3-dev :
CurlClientState juste before curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, $this->onPush, ...

var_dump($this->handle);

=>

.  854 / 4775 ( 17%)
resource(26271) of type (curl_multi)
.resource(26500) of type (curl_multi)
.resource(26569) of type (curl_multi)

adrienfr · 2022-01-19T11:34:21Z

@nicolas-grekas might have found a "leak" in CurlClientState::reset() this block has been removed in recent PRs:

if (\is_resource($this->handle) || $this->handle instanceof \CurlMultiHandle) {
    if (\defined('CURLMOPT_PUSHFUNCTION')) {
        curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, null);
    }

    $active = 0;
    while (\CURLM_CALL_MULTI_PERFORM === curl_multi_exec($this->handle, $active));
}

foreach ($this->openHandles as [$ch]) {
    if (\is_resource($ch) || $ch instanceof \CurlHandle) {
        curl_setopt($ch, \CURLOPT_VERBOSE, false);
    }
}

If I add these 2 blocks (especially the one with the curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, null); call), no more issue. Do you want me to update the PR by re-adding these 2 blocks?

adrienfr · 2022-01-19T11:47:13Z

Regarding this part added in your last PR, I don't really understand the goal but shouldn't we have a call to curl_share_close() somewhere? This block also seems responsible for having a lot more open streams than before (5.3.7)

$this->share = curl_share_init();

curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_DNS);
curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_SSL_SESSION);

if (\defined('CURL_LOCK_DATA_CONNECT')) {
    curl_share_setopt($this->share, \CURLSHOPT_SHARE, \CURL_LOCK_DATA_CONNECT);
}

nicolas-grekas · 2022-01-19T11:49:10Z

I wish I could reproduce locally. If you can share the code privately with me, you can reach me on Symfony's Slack.

nicolas-grekas · 2022-01-19T12:48:24Z

Can you please try this patch instead?

--- a/src/Symfony/Component/HttpClient/Internal/CurlClientState.php
+++ b/src/Symfony/Component/HttpClient/Internal/CurlClientState.php
@@ -23,9 +23,9 @@ use Symfony\Component\HttpClient\Response\CurlResponse;
  */
 final class CurlClientState extends ClientState
 {
-    /** @var \CurlMultiHandle|resource */
+    /** @var \CurlMultiHandle|resource|null */
     public $handle;
-    /** @var \CurlShareHandle|resource */
+    /** @var \CurlShareHandle|resource|null */
     public $share;
     /** @var PushedResponse[] */
     public $pushedResponses = [];
@@ -65,8 +65,17 @@ final class CurlClientState extends ClientState
             return;
         }
 
-        curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, function ($parent, $pushed, array $requestHeaders) use ($maxPendingPushes) {
-            return $this->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
+        // Clone to prevent a circular reference
+        $multi = clone $this;
+        $multi->handle = null;
+        $multi->share = null;
+        $multi->pushedResponses = &$this->pushedResponses;
+        $multi->logger = &$this->logger;
+        $multi->handlesActivity = &$this->handlesActivity;
+        $multi->openHandles = &$this->openHandles;
+
+        curl_multi_setopt($this->handle, \CURLMOPT_PUSHFUNCTION, static function ($parent, $pushed, array $requestHeaders) use ($multi, $maxPendingPushes) {
+            return $multi->handlePush($parent, $pushed, $requestHeaders, $maxPendingPushes);
         });
     }

adrienfr · 2022-01-19T13:35:32Z

Thanks @nicolas-grekas for the patch, it works perfectly, no more issues! 🚀

nicolas-grekas · 2022-01-19T14:00:22Z

Thank you @adrienfr.

carsonbot added Status: Needs Review Bug HttpClient labels Jan 19, 2022

carsonbot added this to the 6.1 milestone Jan 19, 2022

nicolas-grekas changed the base branch from 6.1 to 4.4 January 19, 2022 09:39

nicolas-grekas modified the milestones: 6.1, 4.4 Jan 19, 2022

adrienfr mentioned this pull request Jan 19, 2022

[HttpClient] Failed to open stream: Too many open files #44900

Closed

nicolas-grekas reviewed Jan 19, 2022

View reviewed changes

src/Symfony/Component/HttpClient/Internal/CurlClientState.php Outdated Show resolved Hide resolved

[HttpClient] Fix Failed to open stream: Too many open files

40d22b8

adrienfr force-pushed the ticket_44900 branch from a548fe5 to 40d22b8 Compare January 19, 2022 13:29

nicolas-grekas approved these changes Jan 19, 2022

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels Jan 19, 2022

nicolas-grekas merged commit d0a8092 into symfony:4.4 Jan 19, 2022

This was referenced Jan 28, 2022

Release v4.4.37 #45215

Merged

Release v5.3.14 #45216

Merged

Release v5.4.3 #45217

Merged

Release v6.0.3 #45220

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HttpClient] Fix Failed to open stream: Too many open files #45073

[HttpClient] Fix Failed to open stream: Too many open files #45073

adrienfr commented Jan 19, 2022 •

edited by nicolas-grekas

Loading

adrienfr commented Jan 19, 2022 •

edited

Loading

adrienfr commented Jan 19, 2022 •

edited

Loading

adrienfr commented Jan 19, 2022

nicolas-grekas commented Jan 19, 2022

nicolas-grekas commented Jan 19, 2022

adrienfr commented Jan 19, 2022

nicolas-grekas commented Jan 19, 2022

[HttpClient] Fix Failed to open stream: Too many open files #45073

[HttpClient] Fix Failed to open stream: Too many open files #45073

Conversation

adrienfr commented Jan 19, 2022 • edited by nicolas-grekas Loading

adrienfr commented Jan 19, 2022 • edited Loading

adrienfr commented Jan 19, 2022 • edited Loading

adrienfr commented Jan 19, 2022

nicolas-grekas commented Jan 19, 2022

nicolas-grekas commented Jan 19, 2022

adrienfr commented Jan 19, 2022

nicolas-grekas commented Jan 19, 2022

adrienfr commented Jan 19, 2022 •

edited by nicolas-grekas

Loading

adrienfr commented Jan 19, 2022 •

edited

Loading

adrienfr commented Jan 19, 2022 •

edited

Loading