From 3a02cfb675da68e81c40226135ac4121e72f1378 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Gustavo=20Andr=C3=A9=20dos=20Santos=20Lopes?= Date: Mon, 15 Nov 2010 03:05:32 +0000 Subject: [PATCH] - Added leak_variable() function. - Added mechanism to force outer streams to be closed before their inner ones. - Fixed temp:// streams only handling correctly (through an ad hoc mechanism) reverse closing order when the inner stream is of type memory. --- UPGRADING.INTERNALS | 54 ++++++++++++++++++++++- ext/standard/basic_functions.c | 32 ++++++++++++++ ext/standard/basic_functions.h | 1 + main/php_streams.h | 9 +++- main/streams/memory.c | 18 +++----- main/streams/streams.c | 79 ++++++++++++++++++++++++++++++++-- 6 files changed, 175 insertions(+), 18 deletions(-) diff --git a/UPGRADING.INTERNALS b/UPGRADING.INTERNALS index ababca9e45..525b19a1a5 100644 --- a/UPGRADING.INTERNALS +++ b/UPGRADING.INTERNALS @@ -8,7 +8,9 @@ UPGRADE NOTES - PHP X.Y c. readlink support d. layout of some core ZE structures (zend_op_array, zend_class_entry, ...) e. Zend\zend_fast_cache.h has been removed - f. API Signature changes + f. streams that enclose private streams + g. leak_variable + h. API Signature changes ======================== 1. Internal API changes @@ -68,7 +70,55 @@ ZEND_FAST_FREE_REL(p, fc_type) Use emalloc, emalloc_rel, efree or efree_rel instead. - f. API Signature changes + f. Streams that enclose private streams +Some streams, like the temp:// stream, may enclose private streams. If the +outer stream leaks due to a programming error or is not exposed through a +zval (and therefore is not deleted when all the zvals are gone), it will +be destroyed on shutdown. +The problem is that the outer usually wants itself to close the inner stream, +so that it may do any other shutdown action that requires the inner stream to +be live (e.g. commit data to it). If the outer stream is exposed through a +zval and the inner one isn't, this is not a problem because the outer stream +will be freed when the zval is destroyed, which happens before the resources +are destroyed on shutdown. +On resource list shutdown, the cleanup happens in reverse order of resource +creation, so if the inner stream was created in the opener of the outer stream, +it will be destroyed first. +The following functions were added to the streams API to force a predictable +destruction order: + +PHPAPI php_stream *php_stream_encloses(php_stream *enclosing, php_stream *enclosed); +#define php_stream_free_enclosed(stream_enclosed, close_options) +PHPAPI int _php_stream_free_enclosed(php_stream *stream_enclosed, int close_options TSRMLS_DC); + +Additionally, the following member was added to php_stream: + + struct _php_stream *enclosing_stream; + +and the following macro was added: + +#define PHP_STREAM_FREE_IGNORE_ENCLOSING 32 + +The function php_stream_encloses declares the first stream encloses the second. +This has the effect that, when the inner stream is closed from a resource +destructor it will abort and try to free its enclosing stream instead. +To prevent this from happening when the inner stream is freed from the outer +stream, the macro php_stream_free_enclosed should be used instead of +php_stream_free/php_stream_close/php_stream_pclose, or the flag +PHP_STREAM_FREE_IGNORE_ENCLOSING should be directly passed to php_stream_free. +The outer stream cannot abstain, in its close callback, from either closing the +inner stream or clear the enclosing_stream pointer in its enclosed stream by +calling php_stream_encloses with the 2nd argument NULL. If this is not done, +there will be problems, so observe this requirement when using +php_stream_encloses. + + g. leak_variable +The function leak_variable(variable [, leak_data]) was added. It is only +available on debug builds. It increments the refcount of a zval or, if the +second argument is true and the variable is either an object or a resource +it increments the refcounts of those objects instead. + + h. API Signature changes . zend_list_insert ZEND_API int zend_list_insert(void *ptr, int type TSRMLS_DC); call: zend_list_insert(a, SOMETYPE TSRMLS_CC); diff --git a/ext/standard/basic_functions.c b/ext/standard/basic_functions.c index 930bb30332..a7918abfd6 100644 --- a/ext/standard/basic_functions.c +++ b/ext/standard/basic_functions.c @@ -853,6 +853,11 @@ ZEND_END_ARG_INFO() #if ZEND_DEBUG ZEND_BEGIN_ARG_INFO(arginfo_config_get_hash, 0) ZEND_END_ARG_INFO() + +ZEND_BEGIN_ARG_INFO_EX(arginfo_leak_variable, 0, 0, 1) + ZEND_ARG_INFO(0, variable) + ZEND_ARG_INFO(0, leak_data) +ZEND_END_ARG_INFO() #endif #ifdef HAVE_GETLOADAVG @@ -2997,6 +3002,7 @@ const zend_function_entry basic_functions[] = { /* {{{ */ PHP_FE(parse_ini_string, arginfo_parse_ini_string) #if ZEND_DEBUG PHP_FE(config_get_hash, arginfo_config_get_hash) + PHP_FE(leak_variable, arginfo_leak_variable) #endif PHP_FE(is_uploaded_file, arginfo_is_uploaded_file) PHP_FE(move_uploaded_file, arginfo_move_uploaded_file) @@ -5917,6 +5923,32 @@ PHP_FUNCTION(config_get_hash) /* {{{ */ zend_hash_apply_with_arguments(hash TSRMLS_CC, (apply_func_args_t) add_config_entry_cb, 1, return_value); } /* }}} */ + +/* {{{ proto leak_variable(variable [, leak_data]) */ +PHP_FUNCTION(leak_variable) +{ + zval *zv; + zend_bool leak_data = 0; + + if (zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "z|b", &zv, &leak_data) == FAILURE) { + return; + } + + if (leak_data && (Z_TYPE_P(zv) != IS_RESOURCE && Z_TYPE_P(zv) != IS_OBJECT)) { + php_error_docref0(NULL TSRMLS_CC, E_WARNING, + "Leaking non-zval data is only applicable to resources and objects"); + return; + } + + if (!leak_data) { + zval_add_ref(&zv); + } else if (Z_TYPE_P(zv) == IS_RESOURCE) { + zend_list_addref(Z_RESVAL_P(zv)); + } else if (Z_TYPE_P(zv) == IS_OBJECT) { + Z_OBJ_HANDLER_P(zv, add_ref)(zv TSRMLS_CC); + } +} +/* }}} */ #endif #ifdef HAVE_GETLOADAVG diff --git a/ext/standard/basic_functions.h b/ext/standard/basic_functions.h index 4498e6cf8f..50dbce3c0e 100644 --- a/ext/standard/basic_functions.h +++ b/ext/standard/basic_functions.h @@ -127,6 +127,7 @@ PHP_FUNCTION(parse_ini_file); PHP_FUNCTION(parse_ini_string); #if ZEND_DEBUG PHP_FUNCTION(config_get_hash); +PHP_FUNCTION(leak_variable); #endif PHP_FUNCTION(str_rot13); diff --git a/main/php_streams.h b/main/php_streams.h index 99a75473f4..87da9bc95a 100755 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -225,9 +225,11 @@ struct _php_stream { int eof; #if ZEND_DEBUG - char *open_filename; + const char *open_filename; uint open_lineno; #endif + + struct _php_stream *enclosing_stream; /* this is a private stream owned by enclosing_stream */ }; /* php_stream */ /* state definitions when closing down; these are private to streams.c */ @@ -259,6 +261,10 @@ END_EXTERN_C() #define php_stream_from_zval_no_verify(xstr, ppzval) (xstr) = (php_stream*)zend_fetch_resource((ppzval) TSRMLS_CC, -1, "stream", NULL, 2, php_file_le_stream(), php_file_le_pstream()) BEGIN_EXTERN_C() +PHPAPI php_stream *php_stream_encloses(php_stream *enclosing, php_stream *enclosed); +#define php_stream_free_enclosed(stream_enclosed, close_options) _php_stream_free_enclosed((stream_enclosed), (close_options) TSRMLS_CC) +PHPAPI int _php_stream_free_enclosed(php_stream *stream_enclosed, int close_options TSRMLS_DC); + PHPAPI int php_stream_from_persistent_id(const char *persistent_id, php_stream **stream TSRMLS_DC); #define PHP_STREAM_PERSISTENT_SUCCESS 0 /* id exists */ #define PHP_STREAM_PERSISTENT_FAILURE 1 /* id exists but is not a stream! */ @@ -269,6 +275,7 @@ PHPAPI int php_stream_from_persistent_id(const char *persistent_id, php_stream * #define PHP_STREAM_FREE_PRESERVE_HANDLE 4 /* tell ops->close to not close it's underlying handle */ #define PHP_STREAM_FREE_RSRC_DTOR 8 /* called from the resource list dtor */ #define PHP_STREAM_FREE_PERSISTENT 16 /* manually freeing a persistent connection */ +#define PHP_STREAM_FREE_IGNORE_ENCLOSING 32 /* don't close the enclosing stream instead */ #define PHP_STREAM_FREE_CLOSE (PHP_STREAM_FREE_CALL_DTOR | PHP_STREAM_FREE_RELEASE_STREAM) #define PHP_STREAM_FREE_CLOSE_CASTED (PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_PRESERVE_HANDLE) #define PHP_STREAM_FREE_CLOSE_PERSISTENT (PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_PERSISTENT) diff --git a/main/streams/memory.c b/main/streams/memory.c index b22d0f2193..e2a2a334c6 100644 --- a/main/streams/memory.c +++ b/main/streams/memory.c @@ -42,7 +42,6 @@ typedef struct { size_t fsize; size_t smax; int mode; - php_stream **owner_ptr; } php_stream_memory_data; @@ -112,9 +111,6 @@ static int php_stream_memory_close(php_stream *stream, int close_handle TSRMLS_D if (ms->data && close_handle && ms->mode != TEMP_STREAM_READONLY) { efree(ms->data); } - if (ms->owner_ptr) { - *ms->owner_ptr = NULL; - } efree(ms); return 0; } @@ -301,7 +297,6 @@ PHPAPI php_stream *_php_stream_memory_create(int mode STREAMS_DC TSRMLS_DC) self->fsize = 0; self->smax = ~0u; self->mode = mode; - self->owner_ptr = NULL; stream = php_stream_alloc_rel(&php_stream_memory_ops, self, 0, mode & TEMP_STREAM_READONLY ? "rb" : "w+b"); stream->flags |= PHP_STREAM_FLAG_NO_BUFFER; @@ -376,8 +371,9 @@ static size_t php_stream_temp_write(php_stream *stream, const char *buf, size_t if (memsize + count >= ts->smax) { php_stream *file = php_stream_fopen_tmpfile(); php_stream_write(file, membuf, memsize); - php_stream_close(ts->innerstream); + php_stream_free_enclosed(ts->innerstream, PHP_STREAM_FREE_CLOSE); ts->innerstream = file; + php_stream_encloses(stream, ts->innerstream); } } return php_stream_write(ts->innerstream, buf, count); @@ -415,7 +411,7 @@ static int php_stream_temp_close(php_stream *stream, int close_handle TSRMLS_DC) assert(ts != NULL); if (ts->innerstream) { - ret = php_stream_free(ts->innerstream, PHP_STREAM_FREE_CLOSE | (close_handle ? 0 : PHP_STREAM_FREE_PRESERVE_HANDLE)); + ret = php_stream_free_enclosed(ts->innerstream, PHP_STREAM_FREE_CLOSE | (close_handle ? 0 : PHP_STREAM_FREE_PRESERVE_HANDLE)); } else { ret = 0; } @@ -499,9 +495,10 @@ static int php_stream_temp_cast(php_stream *stream, int castas, void **ret TSRML file = php_stream_fopen_tmpfile(); php_stream_write(file, membuf, memsize); pos = php_stream_tell(ts->innerstream); - - php_stream_close(ts->innerstream); + + php_stream_free_enclosed(ts->innerstream, PHP_STREAM_FREE_CLOSE); ts->innerstream = file; + php_stream_encloses(stream, ts->innerstream); php_stream_seek(ts->innerstream, pos, SEEK_SET); return php_stream_cast(ts->innerstream, castas, ret, 1); @@ -563,8 +560,7 @@ PHPAPI php_stream *_php_stream_temp_create(int mode, size_t max_memory_usage STR stream = php_stream_alloc_rel(&php_stream_temp_ops, self, 0, mode & TEMP_STREAM_READONLY ? "rb" : "w+b"); stream->flags |= PHP_STREAM_FLAG_NO_BUFFER; self->innerstream = php_stream_memory_create_rel(mode); - php_stream_auto_cleanup(self->innerstream); /* do not warn if innerstream is GC'ed before stream */ - ((php_stream_memory_data*)self->innerstream->abstract)->owner_ptr = &self->innerstream; + php_stream_encloses(stream, self->innerstream); return stream; } diff --git a/main/streams/streams.c b/main/streams/streams.c index 54ea92d9a1..b3f40422c3 100755 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -105,6 +105,15 @@ PHP_RSHUTDOWN_FUNCTION(streams) return SUCCESS; } +PHPAPI php_stream *php_stream_encloses(php_stream *enclosing, php_stream *enclosed) +{ + php_stream *orig = enclosed->enclosing_stream; + + php_stream_auto_cleanup(enclosed); + enclosed->enclosing_stream = enclosing; + return orig; +} + PHPAPI int php_stream_from_persistent_id(const char *persistent_id, php_stream **stream TSRMLS_DC) { zend_rsrc_list_entry *le; @@ -272,15 +281,53 @@ fprintf(stderr, "stream_alloc: %s:%p persistent=%s\n", ops->label, ret, persiste ret->rsrc_id = ZEND_REGISTER_RESOURCE(NULL, ret, persistent_id ? le_pstream : le_stream); strlcpy(ret->mode, mode, sizeof(ret->mode)); + ret->wrapper = NULL; + ret->wrapperthis = NULL; + ret->wrapperdata = NULL; + ret->stdiocast = NULL; + ret->orig_path = NULL; + ret->context = NULL; + ret->readbuf = NULL; + ret->enclosing_stream = NULL; + return ret; } /* }}} */ +PHPAPI int _php_stream_free_enclosed(php_stream *stream_enclosed, int close_options TSRMLS_DC) /* {{{ */ +{ + return _php_stream_free(stream_enclosed, + close_options | PHP_STREAM_FREE_IGNORE_ENCLOSING TSRMLS_CC); +} +/* }}} */ + +#if STREAM_DEBUG +static const char *_php_stream_pretty_free_options(int close_options, char *out) +{ + if (close_options & PHP_STREAM_FREE_CALL_DTOR) + strcat(out, "CALL_DTOR, "); + if (close_options & PHP_STREAM_FREE_RELEASE_STREAM) + strcat(out, "RELEASE_STREAM, "); + if (close_options & PHP_STREAM_FREE_PRESERVE_HANDLE) + strcat(out, "PREVERSE_HANDLE, "); + if (close_options & PHP_STREAM_FREE_RSRC_DTOR) + strcat(out, "RSRC_DTOR, "); + if (close_options & PHP_STREAM_FREE_PERSISTENT) + strcat(out, "PERSISTENT, "); + if (close_options & PHP_STREAM_FREE_IGNORE_ENCLOSING) + strcat(out, "IGNORE_ENCLOSING, "); + if (out[0] != '\0') + out[strlen(out) - 2] = '\0'; + return out; +} +#endif + static int _php_stream_free_persistent(zend_rsrc_list_entry *le, void *pStream TSRMLS_DC) { return le->ptr == pStream; } + PHPAPI int _php_stream_free(php_stream *stream, int close_options TSRMLS_DC) /* {{{ */ { int ret = 1; @@ -294,16 +341,40 @@ PHPAPI int _php_stream_free(php_stream *stream, int close_options TSRMLS_DC) /* } #if STREAM_DEBUG -fprintf(stderr, "stream_free: %s:%p[%s] in_free=%d opts=%08x\n", stream->ops->label, stream, stream->orig_path, stream->in_free, close_options); + { + char out[200] = ""; + fprintf(stderr, "stream_free: %s:%p[%s] in_free=%d opts=%s\n", + stream->ops->label, stream, stream->orig_path, stream->in_free, _php_stream_pretty_free_options(close_options, out)); + } + #endif - /* recursion protection */ if (stream->in_free) { - return 1; + /* hopefully called recursively from the enclosing stream; the pointer was NULLed below */ + if ((stream->in_free == 1) && (close_options & PHP_STREAM_FREE_IGNORE_ENCLOSING) && (stream->enclosing_stream == NULL)) { + close_options |= PHP_STREAM_FREE_RSRC_DTOR; /* restore flag */ + } else { + return 1; /* recursion protection */ + } } stream->in_free++; + /* force correct order on enclosing/enclosed stream destruction (only from resource + * destructor as in when reverse destroying the resource list) */ + if ((close_options & PHP_STREAM_FREE_RSRC_DTOR) && + !(close_options & PHP_STREAM_FREE_IGNORE_ENCLOSING) && + (close_options & (PHP_STREAM_FREE_CALL_DTOR | PHP_STREAM_FREE_RELEASE_STREAM)) && /* always? */ + (stream->enclosing_stream != NULL)) { + php_stream *enclosing_stream = stream->enclosing_stream; + stream->enclosing_stream = NULL; + /* we force PHP_STREAM_CALL_DTOR because that's from where the + * enclosing stream can free this stream. We remove rsrc_dtor because + * we want the enclosing stream to be deleted from the resource list */ + return _php_stream_free(enclosing_stream, + (close_options | PHP_STREAM_FREE_CALL_DTOR) & ~PHP_STREAM_FREE_RSRC_DTOR TSRMLS_CC); + } + /* if we are releasing the stream only (and preserving the underlying handle), * we need to do things a little differently. * We are only ever called like this when the stream is cast to a FILE* @@ -404,7 +475,7 @@ fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remov stream->orig_path = NULL; } -# if defined(PHP_WIN32) +# if defined(PHP_WIN32_) OutputDebugString(leakinfo); # else fprintf(stderr, "%s", leakinfo); -- 2.40.0