diff --git a/src/CopyRequest.php b/src/CopyRequest.php index b537fcc..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, @@ -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 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; 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);