Tweak closing

Resource is not nulled in watcher callback if reading/writing fails so close() will still call shutdown/fclose.
amphp · Jun 19, 2017 · 48c4e12 · 48c4e12
1 parent 8d74c85
commit 48c4e12
Show file tree

Hide file tree

Showing 2 changed files with 21 additions and 25 deletions.
diff --git a/lib/ResourceInputStream.php b/lib/ResourceInputStream.php
@@ -25,9 +25,6 @@ final class ResourceInputStream implements InputStream {
     /** @var bool */
     private $readable = true;
 
-    /** @var bool Flag to avoid \fclose() inside destructor */
-    private $inDestructor = false;
-
     /**
      * @param resource $stream Stream resource.
      * @param int $chunkSize Chunk size per `fread()` operation.
@@ -55,16 +52,11 @@ public function __construct($stream, int $chunkSize = self::DEFAULT_CHUNK_SIZE)
         $this->watcher = Loop::onReadable($this->resource, static function ($watcher, $stream) use (
             &$deferred, &$readable, &$resource, $chunkSize
         ) {
-            if ($deferred === null) {
-                return;
-            }
-
             // Error reporting suppressed since fread() produces a warning if the stream unexpectedly closes.
             $data = @\fread($stream, $chunkSize);
 
             if ($data === false || ($data === '' && (!\is_resource($stream) || \feof($stream)))) {
                 $readable = false;
-                $resource = null;
                 Loop::cancel($watcher);
                 $data = null; // Stream closed, resolve read with null.
             }
@@ -106,16 +98,23 @@ public function read(): Promise {
      * @return void
      */
     public function close() {
-        if ($this->resource && !$this->inDestructor) {
+        if ($this->resource) {
             $meta = \stream_get_meta_data($this->resource);
 
-            if (strpos($meta["mode"], "+") !== false) {
+            if (\strpos($meta["mode"], "+") !== false) {
                 \stream_socket_shutdown($this->resource, \STREAM_SHUT_RD);
             } else {
                 \fclose($this->resource);
             }
         }
 
+        $this->free();
+    }
+
+    /**
+     * Nulls reference to resource, marks stream unreadable, and succeeds any pending read with null.
+     */
+    private function free() {
         $this->resource = null;
         $this->readable = false;
 
@@ -136,10 +135,8 @@ public function getResource() {
     }
 
     public function __destruct() {
-        $this->inDestructor = true;
-
         if ($this->resource !== null) {
-            $this->close();
+            $this->free();
         }
     }
 }
diff --git a/lib/ResourceOutputStream.php b/lib/ResourceOutputStream.php
@@ -27,9 +27,6 @@ final class ResourceOutputStream implements OutputStream {
     /** @var int|null */
     private $chunkSize;
 
-    /** @var bool Flag to avoid \fclose() inside destructor */
-    private $inDestructor = false;
-
     /**
      * @param resource $stream Stream resource.
      * @param int|null $chunkSize Chunk size per `fwrite()` operation.
@@ -77,7 +74,6 @@ public function __construct($stream, int $chunkSize = null) {
 
                     if ($written === false || $written === 0) {
                         $writable = false;
-                        $resource = null;
 
                         $message = "Failed to write to socket";
                         if ($error = \error_get_last()) {
@@ -192,9 +188,7 @@ private function send(string $data, bool $end = false): Promise {
         $promise = $deferred->promise();
 
         if ($end) {
-            $promise->onResolve(function () {
-                $this->close();
-            });
+            $promise->onResolve([$this, "close"]);
         }
 
         return $promise;
@@ -206,16 +200,23 @@ private function send(string $data, bool $end = false): Promise {
      * @return void
      */
     public function close() {
-        if ($this->resource && !$this->inDestructor) {
+        if ($this->resource) {
             $meta = \stream_get_meta_data($this->resource);
 
-            if (strpos($meta["mode"], "+") !== false) {
+            if (\strpos($meta["mode"], "+") !== false) {
                 \stream_socket_shutdown($this->resource, \STREAM_SHUT_WR);
             } else {
                 \fclose($this->resource);
             }
         }
 
+        $this->free();
+    }
+
+    /**
+     * Nulls reference to resource, marks stream unwritable, and fails any pending write.
+     */
+    private function free() {
         $this->resource = null;
         $this->writable = false;
 
@@ -239,10 +240,8 @@ public function getResource() {
     }
 
     public function __destruct() {
-        $this->inDestructor = true;
-
         if ($this->resource !== null) {
-            $this->close();
+            $this->free();
         }
     }
 }