Stop closing stderr and stdout streams (php#8569)

Extensions may (and do) write to stderr in mshutdown and similar. In
the best case, with the stderr stream closed, it's just swallowed.

However, some libraries will do things like try to detect color, and
these will outright fail and cause an error path to be taken.

Author: morrisonlevi

NEWS

@@ -2,6 +2,9 @@ PHP                                                                        NEWS
 |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
 ?? ??? 2022, PHP 8.0.20
 
+- CLI:
+  . Fixed bug GH-8575 (CLI closes standard streams too early). (Levi Morrison)
+
 - Core:
   . Fixed Haiku ZTS builds. (David Carlier)
 

ext/zend_test/test.c

@@ -42,6 +42,7 @@ ZEND_BEGIN_MODULE_GLOBALS(zend_test)
 	int observer_show_opcode;
 	int observer_nesting_depth;
 	int replace_zend_execute_ex;
+	zend_bool print_stderr_mshutdown;
 	HashTable global_weakmap;
 ZEND_END_MODULE_GLOBALS(zend_test)
 
@@ -407,6 +408,7 @@ PHP_INI_BEGIN()
 	STD_PHP_INI_BOOLEAN("zend_test.observer.show_init_backtrace", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_show_init_backtrace, zend_zend_test_globals, zend_test_globals)
 	STD_PHP_INI_BOOLEAN("zend_test.observer.show_opcode", "0", PHP_INI_SYSTEM, OnUpdateBool, observer_show_opcode, zend_zend_test_globals, zend_test_globals)
 	STD_PHP_INI_BOOLEAN("zend_test.replace_zend_execute_ex", "0", PHP_INI_SYSTEM, OnUpdateBool, replace_zend_execute_ex, zend_zend_test_globals, zend_test_globals)
+	STD_PHP_INI_BOOLEAN("zend_test.print_stderr_mshutdown", "0", PHP_INI_SYSTEM, OnUpdateBool, print_stderr_mshutdown, zend_zend_test_globals, zend_test_globals)
 PHP_INI_END()
 
 static zend_observer_fcall_handlers observer_fcall_init(zend_execute_data *execute_data);
@@ -526,6 +528,10 @@ PHP_MSHUTDOWN_FUNCTION(zend_test)
 		UNREGISTER_INI_ENTRIES();
 	}
 
+	if (ZT_G(print_stderr_mshutdown)) {
+		fprintf(stderr, "[zend-test] MSHUTDOWN\n");
+	}
+
 	return SUCCESS;
 }
 

ext/zend_test/tests/gh8575.phpt

@@ -0,0 +1,14 @@
+--TEST--
+CLI: stderr is available in mshutdown
+--SKIPIF--
+<?php
+if (!extension_loaded('zend-test')) die('skip zend-test extension required');
+if (php_sapi_name() != "cli") die('skip cli test only');
+?>
+--INI--
+zend_test.print_stderr_mshutdown=1
+--FILE--
+==DONE==
+--EXPECTF--
+==DONE==
+[zend-test] MSHUTDOWN

sapi/cli/php_cli.c

@@ -539,19 +539,21 @@ static void cli_register_file_handles(void) /* {{{ */
 	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_CLOSE;
+	if (s_out) s_out->flags |= PHP_STREAM_FLAG_NO_CLOSE;
+	if (s_err) s_err->flags |= PHP_STREAM_FLAG_NO_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 PHP_DEBUG
-	/* do not close stdout and stderr */
-	s_out->flags |= PHP_STREAM_FLAG_NO_CLOSE;
-	s_err->flags |= PHP_STREAM_FLAG_NO_CLOSE;
-#endif
-
 	s_in_process = s_in;
 
 	php_stream_to_zval(s_in,  &ic.value);