Skip to content

Commit 7f29372

Browse files
committed
- 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.
1 parent 2034e14 commit 7f29372

File tree

4 files changed

+27
-5
lines changed

4 files changed

+27
-5
lines changed

UPGRADING

+2
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,8 @@ UPGRADE NOTES - PHP X.Y
167167
- stream_set_write_buffer() no longer disables the read buffer of a plain
168168
stream when 0 is given as the second argument.
169169
- stream_set_write_buffer() no longer changes the chunk size in socket streams.
170+
- fclose() closes streams with resource refcount > 1; it doesn't merely
171+
decrement the resource refcount.
170172

171173
===================================
172174
5. Changes made to existing methods

ext/standard/file.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -920,7 +920,7 @@ PHPAPI PHP_FUNCTION(fclose)
920920
}
921921

922922
if (!stream->is_persistent) {
923-
zend_list_delete(stream->rsrc_id);
923+
php_stream_close(stream);
924924
} else {
925925
php_stream_pclose(stream);
926926
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
--TEST--
2+
fclose() actually closes streams with refcount > 1
3+
--FILE--
4+
<?php
5+
$s = fopen(__FILE__, "rb");
6+
function separate_zval(&$var) { }
7+
$s2 = $s;
8+
separate_zval($s2);
9+
fclose($s);
10+
echo fread($s2, strlen("<?php"));
11+
echo "\nDone.\n";
12+
--EXPECTF--
13+
Warning: fread(): %d is not a valid stream resource in %s on line %d
14+
15+
Done.

main/streams/streams.c

+9-4
Original file line numberDiff line numberDiff line change
@@ -331,7 +331,6 @@ static int _php_stream_free_persistent(zend_rsrc_list_entry *le, void *pStream T
331331
PHPAPI int _php_stream_free(php_stream *stream, int close_options TSRMLS_DC) /* {{{ */
332332
{
333333
int ret = 1;
334-
int remove_rsrc = 1;
335334
int preserve_handle = close_options & PHP_STREAM_FREE_PRESERVE_HANDLE ? 1 : 0;
336335
int release_cast = 1;
337336
php_stream_context *context = stream->context;
@@ -395,15 +394,21 @@ PHPAPI int _php_stream_free(php_stream *stream, int close_options TSRMLS_DC) /*
395394

396395
#if STREAM_DEBUG
397396
fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remove_rsrc=%d\n",
398-
stream->ops->label, stream, stream->orig_path, preserve_handle, release_cast, remove_rsrc);
397+
stream->ops->label, stream, stream->orig_path, preserve_handle, release_cast,
398+
(close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0);
399399
#endif
400400

401401
/* make sure everything is saved */
402402
_php_stream_flush(stream, 1 TSRMLS_CC);
403403

404404
/* If not called from the resource dtor, remove the stream from the resource list. */
405-
if ((close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0 && remove_rsrc) {
406-
zend_list_delete(stream->rsrc_id);
405+
if ((close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0) {
406+
/* zend_list_delete actually only decreases the refcount; if we're
407+
* releasing the stream, we want to actually delete the resource from
408+
* the resource list, otherwise the resource will point to invalid memory.
409+
* In any case, let's always completely delete it from the resource list,
410+
* not only when PHP_STREAM_FREE_RELEASE_STREAM is set */
411+
while (zend_list_delete(stream->rsrc_id) == SUCCESS) {}
407412
}
408413

409414
/* Remove stream from any context link list */

0 commit comments

Comments
 (0)