- Make fclose() actually close stream, even when the resource refcount is > 1.

  This reverts the fix for bug #24557.
- Make php_stream_free delete the stream from the resources list, not merely
  decrease its refcount, as a single call to zend_list_delete does.
#Not worth the risk merging to 5.3. While change #2 may prevent some segfaults,
#a quick and dirty survey to the codebase only showed calls to php_stream_close
#or php_stream_free on streams allocated in the same function, which would have
#refcount == 1. May be reconsidered.

diff --git a/UPGRADING b/UPGRADING
index 69b58bb709720d01a8879ea1c1556059665b27de..df170bb622b3b226ced879186951682df29bd907 100755 (executable)
--- a/UPGRADING
+++ b/UPGRADING
@@ -167,6 +167,8 @@ UPGRADE NOTES - PHP X.Y
 - stream_set_write_buffer() no longer disables the read buffer of a plain
   stream when 0 is given as the second argument.
 - stream_set_write_buffer() no longer changes the chunk size in socket streams.
+- fclose() closes streams with resource refcount > 1; it doesn't merely
+  decrement the resource refcount.
 
 ===================================
 5. Changes made to existing methods

diff --git a/ext/standard/file.c b/ext/standard/file.c
index f15a0f094303b2f8c2342b88da807e43927c6724..c6b585c2c7567eb85065f38dff032f2c078fb3a0 100644 (file)
--- a/ext/standard/file.c
+++ b/ext/standard/file.c
@@ -920,7 +920,7 @@ PHPAPI PHP_FUNCTION(fclose)
        }
 
        if (!stream->is_persistent) {
-               zend_list_delete(stream->rsrc_id);
+               php_stream_close(stream);
        } else {
                php_stream_pclose(stream);
        }

diff --git a/ext/standard/tests/file/fclose_variation1.phpt b/ext/standard/tests/file/fclose_variation1.phpt
new file mode 100644 (file)
index 0000000..43a6c34
--- /dev/null
+++ b/ext/standard/tests/file/fclose_variation1.phpt
@@ -0,0 +1,15 @@
+--TEST--
+fclose() actually closes streams with refcount > 1
+--FILE--
+<?php
+$s = fopen(__FILE__, "rb");
+function separate_zval(&$var) { }
+$s2 = $s;
+separate_zval($s2);
+fclose($s);
+echo fread($s2, strlen("<?php"));
+echo "\nDone.\n";
+--EXPECTF--
+Warning: fread(): %d is not a valid stream resource in %s on line %d
+
+Done.

diff --git a/main/streams/streams.c b/main/streams/streams.c
index 99f9fdc415e643f328fe7a10fd767dfaf77e3f1c..99cdf26f3d577bc5b855871f365d92072fcb4395 100755 (executable)
--- a/main/streams/streams.c
+++ b/main/streams/streams.c
@@ -331,7 +331,6 @@ static int _php_stream_free_persistent(zend_rsrc_list_entry *le, void *pStream T
 PHPAPI int _php_stream_free(php_stream *stream, int close_options TSRMLS_DC) /* {{{ */
 {
        int ret = 1;
-       int remove_rsrc = 1;
        int preserve_handle = close_options & PHP_STREAM_FREE_PRESERVE_HANDLE ? 1 : 0;
        int release_cast = 1;
        php_stream_context *context = stream->context;
@@ -395,15 +394,21 @@ PHPAPI int _php_stream_free(php_stream *stream, int close_options TSRMLS_DC) /*
 
 #if STREAM_DEBUG
 fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remove_rsrc=%d\n",
-               stream->ops->label, stream, stream->orig_path, preserve_handle, release_cast, remove_rsrc);
+               stream->ops->label, stream, stream->orig_path, preserve_handle, release_cast,
+               (close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0);
 #endif
 
        /* make sure everything is saved */
        _php_stream_flush(stream, 1 TSRMLS_CC);
 
        /* If not called from the resource dtor, remove the stream from the resource list. */
-       if ((close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0 && remove_rsrc) {
-               zend_list_delete(stream->rsrc_id);
+       if ((close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0) {
+               /* zend_list_delete actually only decreases the refcount; if we're
+                * releasing the stream, we want to actually delete the resource from
+                * the resource list, otherwise the resource will point to invalid memory.
+                * In any case, let's always completely delete it from the resource list,
+                * not only when PHP_STREAM_FREE_RELEASE_STREAM is set */
+               while (zend_list_delete(stream->rsrc_id) == SUCCESS) {}
        }
 
        /* Remove stream from any context link list */