From 8b4382607388f029a9b9686655f4f89db3474b6c Mon Sep 17 00:00:00 2001 From: Niklas Keller Date: Wed, 21 Jun 2017 14:07:49 +0200 Subject: [PATCH] Ignore double close --- lib/BlockingHandle.php | 2 +- lib/EioHandle.php | 7 ++++++- lib/ParallelHandle.php | 8 +++++++- lib/UvHandle.php | 1 + test/HandleTest.php | 12 ++++++++++-- 5 files changed, 25 insertions(+), 5 deletions(-) diff --git a/lib/BlockingHandle.php b/lib/BlockingHandle.php index 2529c8a..435877f 100644 --- a/lib/BlockingHandle.php +++ b/lib/BlockingHandle.php @@ -88,7 +88,7 @@ class BlockingHandle implements Handle { */ public function close(): Promise { if ($this->fh === null) { - throw new ClosedException("The file has already been closed"); + return new Success; } $fh = $this->fh; diff --git a/lib/EioHandle.php b/lib/EioHandle.php index 18425b6..0918ac0 100644 --- a/lib/EioHandle.php +++ b/lib/EioHandle.php @@ -180,7 +180,12 @@ class EioHandle implements Handle { private function onClose($deferred, $result, $req) { if ($result === -1) { $error = \eio_get_last_error($req); - $deferred->fail(new StreamException($error)); + if ($error === "Bad file descriptor") { + // Handle is already closed, ignore + $deferred->resolve(); + } else { + $deferred->fail(new StreamException("Closing the file failed: " . $error)); + } } else { $deferred->resolve(); } diff --git a/lib/ParallelHandle.php b/lib/ParallelHandle.php index cd3ef4a..a9fea39 100644 --- a/lib/ParallelHandle.php +++ b/lib/ParallelHandle.php @@ -5,6 +5,7 @@ namespace Amp\File; use Amp\ByteStream\ClosedException; use Amp\ByteStream\StreamException; use Amp\Coroutine; +use Amp\Failure; use Amp\Parallel\Worker\TaskException; use Amp\Parallel\Worker\Worker; use Amp\Parallel\Worker\WorkerException; @@ -73,7 +74,11 @@ class ParallelHandle implements Handle { * {@inheritdoc} */ public function close(): Promise { - $this->open = false; + if (!$this->writable) { + return new Success; + } + + $this->writable = false; if ($this->worker->isRunning()) { $promise = $this->worker->enqueue(new Internal\FileTask('fclose', [], $this->id)); @@ -81,6 +86,7 @@ class ParallelHandle implements Handle { return $promise; } + // FIXME: Should that really return new Success instead of an exception? return new Success; } diff --git a/lib/UvHandle.php b/lib/UvHandle.php index e167d41..ffbf23d 100644 --- a/lib/UvHandle.php +++ b/lib/UvHandle.php @@ -223,6 +223,7 @@ class UvHandle implements Handle { $this->poll->listen($deferred->promise()); \uv_fs_close($this->loop, $this->fh, function ($fh) use ($deferred) { + // FIXME: Check for errors $deferred->resolve(); }); diff --git a/test/HandleTest.php b/test/HandleTest.php index 9d9c5a0..ddcaaf0 100644 --- a/test/HandleTest.php +++ b/test/HandleTest.php @@ -42,8 +42,6 @@ abstract class HandleTest extends TestCase { $path = Fixture::path() . "/write"; /** @var \Amp\File\Handle $handle */ $handle = yield File\open($path, "c+"); - $this->assertSame(0, $handle->tell()); - yield $handle->write("foo"); yield $handle->close(); $this->expectException(ClosedException::class); @@ -51,6 +49,16 @@ abstract class HandleTest extends TestCase { }); } + public function testDoubleClose() { + $this->execute(function () { + $path = Fixture::path() . "/write"; + /** @var \Amp\File\Handle $handle */ + $handle = yield File\open($path, "c+"); + yield $handle->close(); + $this->assertNull(yield $handle->close()); + }); + } + public function testWriteAfterEnd() { $this->execute(function () { $path = Fixture::path() . "/write";