From 0a4a55fd4496523c67e929f2c2be5d11dfdd3de2 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Mon, 20 Jun 2022 14:14:29 +0100 Subject: [PATCH] Allow to not close stream on rscr dtor in php cli sapi --- NEWS | 6 ++++-- UPGRADING | 5 +++++ ext/zend_test/tests/gh8575.phpt | 2 -- main/php_streams.h | 3 +++ main/streams/streams.c | 3 ++- sapi/cli/php_cli.c | 23 ++++++++++++----------- sapi/cli/tests/gh8827-001.phpt | 6 ------ sapi/cli/tests/gh8827-002.phpt | 6 ------ sapi/cli/tests/gh8827-003.phpt | 6 ------ 9 files changed, 26 insertions(+), 34 deletions(-) diff --git a/NEWS b/NEWS index a31efaa79f709..a7f9332b768b7 100644 --- a/NEWS +++ b/NEWS @@ -4,8 +4,10 @@ PHP NEWS - CLI: . Updated the mime-type table for the builtin-server. (Ayesh Karunaratne) - . Fixed potential overflow for the builtin server via the PHP_CLI_SERVER_WORKERS - environment variable. (yiyuaner) + . Fixed potential overflow for the builtin server via the + PHP_CLI_SERVER_WORKERS environment variable. (yiyuaner) + . Fixed GH-8575 by changing STDOUT, STDERR and STDIN to not close on resource + destruction. (Jakub Zelenka) - Core: . Reduced the memory footprint of strings returned by var_export(), diff --git a/UPGRADING b/UPGRADING index c8311a1385b22..8dab1ba232568 100644 --- a/UPGRADING +++ b/UPGRADING @@ -475,6 +475,11 @@ PHP 8.2 UPGRADE NOTES 13. Other Changes ======================================== +- CLI: + . The STDOUT, STDERR and STDIN are no longer closed on resource destruction + which is mostly when the CLI finishes. It is however still possible to + explicitly close those streams using fclose and similar. + - Core: . The iterable type is now a built-in compile time alias for array|Traversable. Error messages relating to iterable will therefore now use array|Traversable. diff --git a/ext/zend_test/tests/gh8575.phpt b/ext/zend_test/tests/gh8575.phpt index a7a6e1d857f21..9830547479f49 100644 --- a/ext/zend_test/tests/gh8575.phpt +++ b/ext/zend_test/tests/gh8575.phpt @@ -1,7 +1,5 @@ --TEST-- CLI: stderr is available in mshutdown ---XFAIL-- -GH-8952 / GH-8833 --SKIPIF-- --EXTENSIONS-- diff --git a/main/php_streams.h b/main/php_streams.h index 8e7bbf6e563be..71ef26c970189 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -185,6 +185,9 @@ struct _php_stream_wrapper { * Currently for internal use only. */ #define PHP_STREAM_FLAG_SUPPRESS_ERRORS 0x100 +/* Do not close handle except it is explicitly closed by user (e.g. fclose) */ +#define PHP_STREAM_FLAG_NO_RSCR_DTOR_CLOSE 0x200 + #define PHP_STREAM_FLAG_WAS_WRITTEN 0x80000000 struct _php_stream { diff --git a/main/streams/streams.c b/main/streams/streams.c index c10af433904bc..bafb2793be1cf 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -383,7 +383,8 @@ PHPAPI int _php_stream_free(php_stream *stream, int close_options) /* {{{ */ context = PHP_STREAM_CONTEXT(stream); - if (stream->flags & PHP_STREAM_FLAG_NO_CLOSE) { + if ((stream->flags & PHP_STREAM_FLAG_NO_CLOSE) || + ((stream->flags & PHP_STREAM_FLAG_NO_RSCR_DTOR_CLOSE) && (close_options & PHP_STREAM_FREE_RSRC_DTOR))) { preserve_handle = 1; } diff --git a/sapi/cli/php_cli.c b/sapi/cli/php_cli.c index 152b97a39c1aa..9830d84a0afb0 100644 --- a/sapi/cli/php_cli.c +++ b/sapi/cli/php_cli.c @@ -526,7 +526,7 @@ static void php_cli_usage(char *argv0) static php_stream *s_in_process = NULL; -static void cli_register_file_handles(bool no_close) /* {{{ */ +static void cli_register_file_handles(void) { php_stream *s_in, *s_out, *s_err; php_stream_context *sc_in=NULL, *sc_out=NULL, *sc_err=NULL; @@ -536,6 +536,14 @@ static void cli_register_file_handles(bool no_close) /* {{{ */ s_out = php_stream_open_wrapper_ex("php://stdout", "wb", 0, NULL, sc_out); s_err = php_stream_open_wrapper_ex("php://stderr", "wb", 0, NULL, sc_err); + /* Release stream resources, but don't free the underlying handles. Othewrise, + * extensions which write to stderr or company during mshutdown/gshutdown + * won't have the expected functionality. + */ + if (s_in) s_in->flags |= PHP_STREAM_FLAG_NO_RSCR_DTOR_CLOSE; + if (s_out) s_out->flags |= PHP_STREAM_FLAG_NO_RSCR_DTOR_CLOSE; + if (s_err) s_err->flags |= PHP_STREAM_FLAG_NO_RSCR_DTOR_CLOSE; + if (s_in==NULL || s_out==NULL || s_err==NULL) { if (s_in) php_stream_close(s_in); if (s_out) php_stream_close(s_out); @@ -543,12 +551,6 @@ static void cli_register_file_handles(bool no_close) /* {{{ */ return; } - if (no_close) { - s_in->flags |= PHP_STREAM_FLAG_NO_CLOSE; - s_out->flags |= PHP_STREAM_FLAG_NO_CLOSE; - s_err->flags |= PHP_STREAM_FLAG_NO_CLOSE; - } - s_in_process = s_in; php_stream_to_zval(s_in, &ic.value); @@ -567,7 +569,6 @@ static void cli_register_file_handles(bool no_close) /* {{{ */ ec.name = zend_string_init_interned("STDERR", sizeof("STDERR")-1, 0); zend_register_constant(&ec); } -/* }}} */ static const char *param_mode_conflict = "Either execute direct code, process stdin or use a file.\n"; @@ -954,7 +955,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ switch (behavior) { case PHP_MODE_STANDARD: if (script_file) { - cli_register_file_handles(/* no_close */ PHP_DEBUG || num_repeats > 1); + cli_register_file_handles(); } if (interactive) { @@ -990,7 +991,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ } break; case PHP_MODE_CLI_DIRECT: - cli_register_file_handles(/* no_close */ PHP_DEBUG || num_repeats > 1); + cli_register_file_handles(); zend_eval_string_ex(exec_direct, NULL, "Command line code", 1); break; @@ -1005,7 +1006,7 @@ static int do_cli(int argc, char **argv) /* {{{ */ file_handle.filename = NULL; } - cli_register_file_handles(/* no_close */ PHP_DEBUG || num_repeats > 1); + cli_register_file_handles(); if (exec_begin) { zend_eval_string_ex(exec_begin, NULL, "Command line begin code", 1); diff --git a/sapi/cli/tests/gh8827-001.phpt b/sapi/cli/tests/gh8827-001.phpt index fab7b02d9e975..d852439f4bae2 100644 --- a/sapi/cli/tests/gh8827-001.phpt +++ b/sapi/cli/tests/gh8827-001.phpt @@ -8,12 +8,6 @@ if (php_sapi_name() != "cli") { if (PHP_OS_FAMILY == 'Windows') { die("skip not for Windows"); } -if (PHP_DEBUG) { - die("skip std streams are not closeable in debug builds"); -} -if (getenv('SKIP_REPEAT')) { - die("skip cannot be repeated"); -} ?> --FILE-- --FILE-- --FILE--