Fire open observer end handlers after a zend_bailout

Closes GH-6377

diff --git a/Zend/zend_observer.c b/Zend/zend_observer.c
index 9c2d1cdf51c4e15999671140d0fd3598f6df360f..a8ce1eb5c057faa86c23e7ee44e48d4f379f1ed6 100644 (file)
--- a/Zend/zend_observer.c
+++ b/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)

diff --git a/Zend/zend_observer.h b/Zend/zend_observer.h
index 1d20306a170186a4e6a6cbd3e86125206634d54e..cb29729ec45da14750029856b67e4a392bfe6eaf 100644 (file)
--- a/Zend/zend_observer.h
+++ b/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);

diff --git a/ext/zend_test/tests/observer_error_01.phpt b/ext/zend_test/tests/observer_error_01.phpt
new file mode 100644 (file)
index 0000000..5ea619f
--- /dev/null
+++ b/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'>

diff --git a/ext/zend_test/tests/observer_error_02.phpt b/ext/zend_test/tests/observer_error_02.phpt
new file mode 100644 (file)
index 0000000..959544e
--- /dev/null
+++ b/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'>

diff --git a/ext/zend_test/tests/observer_error_03.phpt b/ext/zend_test/tests/observer_error_03.phpt
new file mode 100644 (file)
index 0000000..3d8150a
--- /dev/null
+++ b/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'>

diff --git a/ext/zend_test/tests/observer_error_04.phpt b/ext/zend_test/tests/observer_error_04.phpt
new file mode 100644 (file)
index 0000000..ca2532a
--- /dev/null
+++ b/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'>

diff --git a/main/main.c b/main/main.c
index 60fdb89efe40ac4d76709c103830a7c373c4fc92..d2d19a5ee828e6f497e9c1586751d2fec52b41a7 100644 (file)
--- a/main/main.c
+++ b/main/main.c
@@ -1740,6 +1740,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();