[Observer] Fire open observer end handlers after a zend_bailout

Zend/zend_observer.c

@@ -44,11 +44,13 @@ zend_llist zend_observer_error_callbacks;
 int zend_observer_fcall_op_array_extension = -1;
 
 ZEND_TLS zend_arena *fcall_handlers_arena = NULL;
+ZEND_TLS zend_execute_data *first_observed_frame = NULL;
+ZEND_TLS zend_execute_data *current_observed_frame = NULL;
 
 // Call during minit/startup ONLY
 ZEND_API void zend_observer_fcall_register(zend_observer_fcall_init init) {
-	/* We don't want to get an extension handle unless an ext installs an observer */
 	if (!ZEND_OBSERVER_ENABLED) {
+		/* We don't want to get an extension handle unless an ext installs an observer */
 		zend_observer_fcall_op_array_extension =
 			zend_get_op_array_extension_handle("Zend Observer");
 
@@ -160,6 +162,11 @@ static void ZEND_FASTCALL _zend_observe_fcall_begin(zend_execute_data *execute_d
 		return;
 	}
 
+	if (first_observed_frame == NULL) {
+		first_observed_frame = execute_data;
+	}
+	current_observed_frame = execute_data;
+
 	end = fcall_data->end;
 	for (handlers = fcall_data->handlers; handlers != end; ++handlers) {
 		if (handlers->begin) {
@@ -208,6 +215,25 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
 			handlers->end(execute_data, return_value);
 		}
 	}
+
+	if (first_observed_frame == execute_data) {
+		first_observed_frame = NULL;
+		current_observed_frame = NULL;
+	} else {
+		current_observed_frame = execute_data->prev_execute_data;
+	}
+}
+
+ZEND_API void zend_observer_fcall_end_all(void)
+{
+	zend_execute_data *ex = current_observed_frame;
+	while (ex != NULL) {
+		if (ex->func->type != ZEND_INTERNAL_FUNCTION) {
+			zend_observer_fcall_end(ex, NULL);
+		}
+		ex = ex->prev_execute_data;
+	}
+	current_observed_frame = NULL;
 }
 
 ZEND_API void zend_observer_error_register(zend_observer_error_cb cb)

Zend/zend_observer.h

@@ -70,6 +70,8 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
 	zend_execute_data *execute_data,
 	zval *return_value);
 
+ZEND_API void zend_observer_fcall_end_all(void);
+
 typedef void (*zend_observer_error_cb)(int type, const char *error_filename, uint32_t error_lineno, zend_string *message);
 
 ZEND_API void zend_observer_error_register(zend_observer_error_cb callback);

ext/zend_test/tests/observer_error_01.phpt

@@ -0,0 +1,29 @@
+--TEST--
+Observer: End handlers fire after a fatal error
+--SKIPIF--
+<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
+--INI--
+zend_test.observer.enabled=1
+zend_test.observer.observe_all=1
+zend_test.observer.show_return_value=1
+memory_limit=1M
+--FILE--
+<?php
+function foo()
+{
+    str_repeat('.', 1024 * 1024 * 2); // 2MB
+}
+
+foo();
+
+echo 'You should not see this.';
+?>
+--EXPECTF--
+<!-- init '%s/observer_error_%d.php' -->
+<file '%s/observer_error_%d.php'>
+  <!-- init foo() -->
+  <foo>
+
+Fatal error: Allowed memory size of 2097152 bytes exhausted%s(tried to allocate %d bytes) in %s on line %d
+  </foo:NULL>
+</file '%s/observer_error_%d.php'>

ext/zend_test/tests/observer_error_02.phpt

@@ -0,0 +1,28 @@
+--TEST--
+Observer: End handlers fire after a userland fatal error
+--SKIPIF--
+<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
+--INI--
+zend_test.observer.enabled=1
+zend_test.observer.observe_all=1
+zend_test.observer.show_return_value=1
+--FILE--
+<?php
+function foo()
+{
+    trigger_error('Foo error', E_USER_ERROR);
+}
+
+foo();
+
+echo 'You should not see this.';
+?>
+--EXPECTF--
+<!-- init '%s/observer_error_%d.php' -->
+<file '%s/observer_error_%d.php'>
+  <!-- init foo() -->
+  <foo>
+
+Fatal error: Foo error in %s on line %d
+  </foo:NULL>
+</file '%s/observer_error_%d.php'>

ext/zend_test/tests/observer_error_03.phpt

@@ -0,0 +1,39 @@
+--TEST--
+Observer: non-fatal errors do not fire end handlers prematurely
+--SKIPIF--
+<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
+--INI--
+zend_test.observer.enabled=1
+zend_test.observer.observe_all=1
+zend_test.observer.show_return_value=1
+--FILE--
+<?php
+function foo()
+{
+    return $this_does_not_exit; // E_WARNING
+}
+
+function main()
+{
+    foo();
+    echo 'After error.' . PHP_EOL;
+}
+
+main();
+
+echo 'Done.' . PHP_EOL;
+?>
+--EXPECTF--
+<!-- init '%s/observer_error_%d.php' -->
+<file '%s/observer_error_%d.php'>
+  <!-- init main() -->
+  <main>
+    <!-- init foo() -->
+    <foo>
+
+Warning: Undefined variable $this_does_not_exit in %s on line %d
+    </foo:NULL>
+After error.
+  </main:NULL>
+Done.
+</file '%s/observer_error_%d.php'>

ext/zend_test/tests/observer_error_04.phpt

@@ -0,0 +1,46 @@
+--TEST--
+Observer: fatal errors caught with zend_try will not fire end handlers prematurely
+--SKIPIF--
+<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
+<?php if (!extension_loaded('soap')) die('skip: soap extension required'); ?>
+--INI--
+zend_test.observer.enabled=1
+zend_test.observer.observe_all=1
+zend_test.observer.show_return_value=1
+--FILE--
+<?php
+function foo()
+{
+    // ext/soap catches a zend_bailout and then throws an exception
+    $client = new SoapClient('foo');
+}
+
+function main()
+{
+    foo();
+}
+
+// try/catch is on main() to ensure ZEND_HANDLE_EXCEPTION does not fire the end handlers again
+try {
+    main();
+} catch (SoapFault $e) {
+    echo $e->getMessage() . PHP_EOL;
+}
+
+echo 'Done.' . PHP_EOL;
+?>
+--EXPECTF--
+<!-- init '%s/observer_error_%d.php' -->
+<file '%s/observer_error_%d.php'>
+  <!-- init main() -->
+  <main>
+    <!-- init foo() -->
+    <foo>
+      <!-- Exception: SoapFault -->
+    </foo:NULL>
+    <!-- Exception: SoapFault -->
+  </main:NULL>
+SOAP-ERROR: Parsing WSDL: Couldn't load from 'foo' : failed to load external entity "foo"
+
+Done.
+</file '%s/observer_error_%d.php'>

main/main.c

@@ -1747,6 +1747,11 @@ void php_request_shutdown(void *dummy)
 
 	php_deactivate_ticks();
 
+	/* 0. Call any open observer end handlers that are still open after a zend_bailout */
+	if (ZEND_OBSERVER_ENABLED) {
+		zend_observer_fcall_end_all();
+	}
+
 	/* 1. Call all possible shutdown functions registered with register_shutdown_function() */
 	if (PG(modules_activated)) {
 		php_call_shutdown_functions();