svn: /php/php-src/trunk/ UPGRADING ext/standard/file.c ext/standard/tests/file/fclose_variation1.phpt main/streams/streams.c

cataphract Mon, 21 Mar 2011 02:58:54 +0000 Revision: http://svn.php.net/viewvc?view=revision&revision=309491 Log: - 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. Bugs: http://bugs.php.net/24557 (Closed) streams SIG_SEGVs http://bugs.php.net/2 (Closed) hash_environment Changed paths: U php/php-src/trunk/UPGRADING U php/php-src/trunk/ext/standard/file.c A php/php-src/trunk/ext/standard/tests/file/fclose_variation1.phpt U php/php-src/trunk/main/streams/streams.c Modified: php/php-src/trunk/UPGRADING =================================================================== --- php/php-src/trunk/UPGRADING 2011-03-21 00:07:47 UTC (rev 309490) +++ php/php-src/trunk/UPGRADING 2011-03-21 02:58:54 UTC (rev 309491) @@ -167,6 +167,8 @@ - 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 Modified: php/php-src/trunk/ext/standard/file.c =================================================================== --- php/php-src/trunk/ext/standard/file.c 2011-03-21 00:07:47 UTC (rev 309490) +++ php/php-src/trunk/ext/standard/file.c 2011-03-21 02:58:54 UTC (rev 309491) @@ -920,7 +920,7 @@ } if (!stream->is_persistent) { - zend_list_delete(stream->rsrc_id); + php_stream_close(stream); } else { php_stream_pclose(stream); } Added: php/php-src/trunk/ext/standard/tests/file/fclose_variation1.phpt =================================================================== --- php/php-src/trunk/ext/standard/tests/file/fclose_variation1.phpt (rev 0) +++ php/php-src/trunk/ext/standard/tests/file/fclose_variation1.phpt 2011-03-21 02:58:54 UTC (rev 309491) @@ -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. Modified: php/php-src/trunk/main/streams/streams.c =================================================================== --- php/php-src/trunk/main/streams/streams.c 2011-03-21 00:07:47 UTC (rev 309490) +++ php/php-src/trunk/main/streams/streams.c 2011-03-21 02:58:54 UTC (rev 309491) @@ -331,7 +331,6 @@ 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 @@ #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 */