From 1c001680e062ab2140891c374ccb7f9f858b36ff Mon Sep 17 00:00:00 2001 From: Jason Woods Date: Tue, 8 Sep 2020 10:39:18 +0100 Subject: [PATCH 1/3] Fix Too many files open errors with large composer setups --- src/CopyRequest.php | 17 ++++++++++------- src/CurlMulti.php | 5 ++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/src/CopyRequest.php b/src/CopyRequest.php index b537fcc..b598b7d 100644 --- a/src/CopyRequest.php +++ b/src/CopyRequest.php @@ -69,6 +69,16 @@ public function makeSuccess() */ public function getCurlOptions() { + if ($this->fp) { + fclose($this->fp); + } + $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 +97,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 index cbc7a76..929831d 100644 --- 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; From ebf5b7bddfd257505637564ac0f6f6c61e120d38 Mon Sep 17 00:00:00 2001 From: Jason Woods Date: Tue, 8 Sep 2020 11:29:04 +0100 Subject: [PATCH 2/3] Ensure only cloned copy removes failures --- src/CopyRequest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/CopyRequest.php b/src/CopyRequest.php index b598b7d..e6aac74 100644 --- a/src/CopyRequest.php +++ b/src/CopyRequest.php @@ -17,7 +17,7 @@ class CopyRequest extends BaseRequest /** @var resource> */ private $fp; - private $success = false; + private $success = true; protected static $defaultCurlOptions = array( CURLOPT_HTTPGET => true, @@ -72,6 +72,7 @@ public function getCurlOptions() if ($this->fp) { fclose($this->fp); } + $this->success = false; $this->fp = fopen($this->destination, 'wb'); if (!$this->fp) { throw new FetchException( From f095efb72943fd3e25763599981edbf7b100e983 Mon Sep 17 00:00:00 2001 From: Jason Woods Date: Tue, 8 Sep 2020 11:56:33 +0100 Subject: [PATCH 3/3] Update tests --- tests/unit/CopyRequestTest.php | 9 +++++---- tests/unit/PrefetcherTest.php | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/tests/unit/CopyRequestTest.php b/tests/unit/CopyRequestTest.php index d1a782b..6bce6b6 100644 --- a/tests/unit/CopyRequestTest.php +++ b/tests/unit/CopyRequestTest.php @@ -60,10 +60,11 @@ 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 @@ -71,8 +72,8 @@ public function testDestruct() 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 index fb83b9b..5e0c08f 100644 --- 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("37: Couldn't open file /uso800.txt"), true, IOInterface::NORMAL)->shouldBeCalledTimes(1); $this->iop->writeError(" Finished: success: 0, skipped: 0, failure: 1, total: 1", true, IOInterface::NORMAL)->shouldBeCalledTimes(1);