Merge pull request #231 from driskell/fix-too-many-open-files

Fix Too many files open errors with large composer setups
nexcess · Oct 28, 2020 · 85844c9 · 85844c9
2 parents 541bce4 + f095efb
commit 85844c9
Show file tree

Hide file tree

Showing 4 changed files with 22 additions and 14 deletions.
diff --git a/src/CopyRequest.php b/src/CopyRequest.php
@@ -17,7 +17,7 @@ class CopyRequest extends BaseRequest
     /** @var resource<stream<plainfile>> */
     private $fp;
 
-    private $success = false;
+    private $success = true;
 
     protected static $defaultCurlOptions = array(
         CURLOPT_HTTPGET => true,
@@ -69,6 +69,17 @@ public function makeSuccess()
      */
     public function getCurlOptions()
     {
+        if ($this->fp) {
+            fclose($this->fp);
+        }
+        $this->success = false;
+        $this->fp = fopen($this->destination, 'wb');
+        if (!$this->fp) {
+            throw new FetchException(
+                'The file could not be written to ' . $this->destination
+            );
+        }
+
         $curlOpts = parent::getCurlOptions();
         $curlOpts[CURLOPT_FILE] = $this->fp;
         return $curlOpts;
@@ -87,13 +98,6 @@ public function setDestination($destination)
         }
 
         $this->createDir($destination);
-
-        $this->fp = fopen($destination, 'wb');
-        if (!$this->fp) {
-            throw new FetchException(
-                'The file could not be written to ' . $destination
-            );
-        }
     }
 
     private function createDir($fileName)

diff --git a/src/CurlMulti.php b/src/CurlMulti.php
@@ -88,7 +88,10 @@ public function setRequests(array $requests)
     public function setupEventLoop()
     {
         while (count($this->unused) > 0 && count($this->requests) > 0) {
-            $request = array_pop($this->requests);
+            // Process request using a clone to ensure any memory we cause it to allocate is freed after we unset
+            // Otherwise, memory is still held for all requests due to it existing on the callstack
+            // Callers should not need to know this though
+            $request = clone array_pop($this->requests);
             $ch = array_pop($this->unused);
             $index = (int)$ch;
 

diff --git a/tests/unit/CopyRequestTest.php b/tests/unit/CopyRequestTest.php
@@ -60,19 +60,20 @@ public function testDestruct()
         $tmpfile = tempnam(sys_get_temp_dir(), 'composer_unit_test_');
 
         $req = new CopyRequest('http://example.com/', $tmpfile, false, $this->iop->reveal(), $this->configp->reveal());
+        $req->getCurlOptions();
+        $req->makeSuccess();
         $this->assertFileExists($tmpfile);
 
-        // if $req->success === true ...
-        $req->makeSuccess();
+        // if $req->success === true (default, or on success) ...
         unset($req);
 
         // then tmpfile remain
         $this->assertFileExists($tmpfile);
         unlink($tmpfile);
 
         $req = new CopyRequest('http://example.com/', $tmpfile, false, $this->iop->reveal(), $this->configp->reveal());
-        // if $req->success === false (default) ...
-        // $req->makeSuccess();
+        // if $req->success === false (flagged when download starts) ...
+        $req->getCurlOptions();
         unset($req);
 
         // then cleaned tmpfile automatically

diff --git a/tests/unit/PrefetcherTest.php b/tests/unit/PrefetcherTest.php
@@ -26,7 +26,7 @@ public function testFetchAllOnFailure()
             CURLOPT_URL => 'file://uso800.txt',
             CURLOPT_FILE => tmpfile(),
         ));
-     
+
         $this->iop->writeError(arg::containingString("<warning>37: Couldn't open file /uso800.txt</warning>"), true, IOInterface::NORMAL)->shouldBeCalledTimes(1);
         $this->iop->writeError("    Finished: <comment>success: 0, skipped: 0, failure: 1, total: 1</comment>", true, IOInterface::NORMAL)->shouldBeCalledTimes(1);