Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to not close stream on rscr dtor in php cli sapi #8833

Merged
merged 1 commit into from
Jul 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
5 changes: 5 additions & 0 deletions UPGRADING
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 0 additions & 2 deletions ext/zend_test/tests/gh8575.phpt
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
--TEST--
CLI: stderr is available in mshutdown
--XFAIL--
GH-8952 / GH-8833
--SKIPIF--
<?php if (php_sapi_name() != "cli") die('skip cli test only'); ?>
--EXTENSIONS--
Expand Down
3 changes: 3 additions & 0 deletions main/php_streams.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion main/streams/streams.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
23 changes: 12 additions & 11 deletions sapi/cli/php_cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -536,19 +536,21 @@ 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);
if (s_err) php_stream_close(s_err);
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);
Expand All @@ -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";

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;

Expand All @@ -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);
Expand Down
6 changes: 0 additions & 6 deletions sapi/cli/tests/gh8827-001.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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--
<?php
Expand Down
6 changes: 0 additions & 6 deletions sapi/cli/tests/gh8827-002.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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--
<?php
Expand Down
6 changes: 0 additions & 6 deletions sapi/cli/tests/gh8827-003.phpt
Original file line number Diff line number Diff line change
Expand Up @@ -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--
<?php
Expand Down