Skip to content

Commit 194073a

Browse files
SammyKSammyK
committed
Fire open observer end handlers after a zend_bailout
1 parent 6768f47 commit 194073a

File tree

7 files changed

+177
-0
lines changed

7 files changed

+177
-0
lines changed

‎Zend/zend_observer.c

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,19 @@ typedef struct _zend_observer_fcall_data {
3838
zend_observer_fcall_handlers handlers[1];
3939
} zend_observer_fcall_data;
4040

41+
typedef struct _zend_observer_frame {
42+
zend_execute_data *execute_data;
43+
struct _zend_observer_frame *prev;
44+
} zend_observer_frame;
45+
4146
zend_llist zend_observers_fcall_list;
4247
zend_llist zend_observer_error_callbacks;
4348

4449
int zend_observer_fcall_op_array_extension = -1;
4550

4651
ZEND_TLS zend_arena *fcall_handlers_arena = NULL;
52+
ZEND_TLS zend_arena *observed_stack_arena = NULL;
53+
ZEND_TLS zend_observer_frame *current_observed_frame = NULL;
4754

4855
// Call during minit/startup ONLY
4956
ZEND_API void zend_observer_fcall_register(zend_observer_fcall_init init) {
@@ -75,12 +82,15 @@ ZEND_API void zend_observer_startup(void) {
7582
ZEND_API void zend_observer_activate(void) {
7683
if (ZEND_OBSERVER_ENABLED) {
7784
fcall_handlers_arena = zend_arena_create(4096);
85+
// TODO: How big should the arena be?
86+
observed_stack_arena = zend_arena_create(2048);
7887
}
7988
}
8089

8190
ZEND_API void zend_observer_deactivate(void) {
8291
if (fcall_handlers_arena) {
8392
zend_arena_destroy(fcall_handlers_arena);
93+
zend_arena_destroy(observed_stack_arena);
8494
}
8595
}
8696

@@ -137,6 +147,7 @@ static void ZEND_FASTCALL _zend_observe_fcall_begin(zend_execute_data *execute_d
137147
uint32_t fn_flags;
138148
zend_observer_fcall_data *fcall_data;
139149
zend_observer_fcall_handlers *handlers, *end;
150+
zend_observer_frame *frame;
140151

141152
if (!ZEND_OBSERVER_ENABLED) {
142153
return;
@@ -160,6 +171,11 @@ static void ZEND_FASTCALL _zend_observe_fcall_begin(zend_execute_data *execute_d
160171
return;
161172
}
162173

174+
frame = zend_arena_alloc(&observed_stack_arena, sizeof(zend_observer_frame));
175+
frame->execute_data = execute_data;
176+
frame->prev = current_observed_frame;
177+
current_observed_frame = frame;
178+
163179
end = fcall_data->end;
164180
for (handlers = fcall_data->handlers; handlers != end; ++handlers) {
165181
if (handlers->begin) {
@@ -188,6 +204,7 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
188204
zend_function *func = execute_data->func;
189205
zend_observer_fcall_data *fcall_data;
190206
zend_observer_fcall_handlers *handlers, *end;
207+
zend_observer_frame *frame;
191208

192209
if (!ZEND_OBSERVER_ENABLED
193210
|| !ZEND_OBSERVABLE_FN(func->common.fn_flags)) {
@@ -208,6 +225,17 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
208225
handlers->end(execute_data, return_value);
209226
}
210227
}
228+
229+
frame = current_observed_frame;
230+
current_observed_frame = frame->prev;
231+
zend_arena_release(&observed_stack_arena, frame);
232+
}
233+
234+
ZEND_API void zend_observer_fcall_end_all(void)
235+
{
236+
while (current_observed_frame != NULL) {
237+
zend_observer_fcall_end(current_observed_frame->execute_data, NULL);
238+
}
211239
}
212240

213241
ZEND_API void zend_observer_error_register(zend_observer_error_cb cb)

‎Zend/zend_observer.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ ZEND_API void ZEND_FASTCALL zend_observer_fcall_end(
7070
zend_execute_data *execute_data,
7171
zval *return_value);
7272

73+
ZEND_API void zend_observer_fcall_end_all(void);
74+
7375
typedef void (*zend_observer_error_cb)(int type, const char *error_filename, uint32_t error_lineno, zend_string *message);
7476

7577
ZEND_API void zend_observer_error_register(zend_observer_error_cb callback);

‎ext/zend_test/tests/observer_error_01.phpt

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
Observer: End handlers fire after a fatal error
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.observe_all=1
8+
zend_test.observer.show_return_value=1
9+
memory_limit=1M
10+
--FILE--
11+
<?php
12+
function foo()
13+
{
14+
str_repeat('.', 1024 * 1024 * 2); // 2MB
15+
}
16+
17+
foo();
18+
19+
echo 'You should not see this.';
20+
?>
21+
--EXPECTF--
22+
<!-- init '%s/observer_error_%d.php' -->
23+
<file '%s/observer_error_%d.php'>
24+
<!-- init foo() -->
25+
<foo>
26+
27+
Fatal error: Allowed memory size of 2097152 bytes exhausted%s(tried to allocate %d bytes) in %s on line %d
28+
</foo:NULL>
29+
</file '%s/observer_error_%d.php'>

‎ext/zend_test/tests/observer_error_02.phpt

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
--TEST--
2+
Observer: End handlers fire after a userland fatal error
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.observe_all=1
8+
zend_test.observer.show_return_value=1
9+
--FILE--
10+
<?php
11+
function foo()
12+
{
13+
trigger_error('Foo error', E_USER_ERROR);
14+
}
15+
16+
foo();
17+
18+
echo 'You should not see this.';
19+
?>
20+
--EXPECTF--
21+
<!-- init '%s/observer_error_%d.php' -->
22+
<file '%s/observer_error_%d.php'>
23+
<!-- init foo() -->
24+
<foo>
25+
26+
Fatal error: Foo error in %s on line %d
27+
</foo:NULL>
28+
</file '%s/observer_error_%d.php'>

‎ext/zend_test/tests/observer_error_03.phpt

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
Observer: non-fatal errors do not fire end handlers prematurely
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
--INI--
6+
zend_test.observer.enabled=1
7+
zend_test.observer.observe_all=1
8+
zend_test.observer.show_return_value=1
9+
--FILE--
10+
<?php
11+
function foo()
12+
{
13+
return $this_does_not_exit; // E_WARNING
14+
}
15+
16+
function main()
17+
{
18+
foo();
19+
echo 'After error.' . PHP_EOL;
20+
}
21+
22+
main();
23+
24+
echo 'Done.' . PHP_EOL;
25+
?>
26+
--EXPECTF--
27+
<!-- init '%s/observer_error_%d.php' -->
28+
<file '%s/observer_error_%d.php'>
29+
<!-- init main() -->
30+
<main>
31+
<!-- init foo() -->
32+
<foo>
33+
34+
Warning: Undefined variable $this_does_not_exit in %s on line %d
35+
</foo:NULL>
36+
After error.
37+
</main:NULL>
38+
Done.
39+
</file '%s/observer_error_%d.php'>

‎ext/zend_test/tests/observer_error_04.phpt

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
--TEST--
2+
Observer: fatal errors caught with zend_try will not fire end handlers prematurely
3+
--SKIPIF--
4+
<?php if (!extension_loaded('zend-test')) die('skip: zend-test extension required'); ?>
5+
<?php if (!extension_loaded('soap')) die('skip: soap extension required'); ?>
6+
--INI--
7+
zend_test.observer.enabled=1
8+
zend_test.observer.observe_all=1
9+
zend_test.observer.show_return_value=1
10+
--FILE--
11+
<?php
12+
function foo()
13+
{
14+
// ext/soap catches a zend_bailout and then throws an exception
15+
$client = new SoapClient('foo');
16+
}
17+
18+
function main()
19+
{
20+
foo();
21+
}
22+
23+
// try/catch is on main() to ensure ZEND_HANDLE_EXCEPTION does not fire the end handlers again
24+
try {
25+
main();
26+
} catch (SoapFault $e) {
27+
echo $e->getMessage() . PHP_EOL;
28+
}
29+
30+
echo 'Done.' . PHP_EOL;
31+
?>
32+
--EXPECTF--
33+
<!-- init '%s/observer_error_%d.php' -->
34+
<file '%s/observer_error_%d.php'>
35+
<!-- init main() -->
36+
<main>
37+
<!-- init foo() -->
38+
<foo>
39+
<!-- Exception: SoapFault -->
40+
</foo:NULL>
41+
<!-- Exception: SoapFault -->
42+
</main:NULL>
43+
SOAP-ERROR: Parsing WSDL: Couldn't load from 'foo' : failed to load external entity "foo"
44+
45+
Done.
46+
</file '%s/observer_error_%d.php'>

‎main/main.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1752,6 +1752,11 @@ void php_request_shutdown(void *dummy)
17521752
php_call_shutdown_functions();
17531753
}
17541754

1755+
/* 1.5. Call any open end handlers that are still open after a zend_bailout */
1756+
if (ZEND_OBSERVER_ENABLED) {
1757+
zend_observer_fcall_end_all();
1758+
}
1759+
17551760
/* 2. Call all possible __destruct() functions */
17561761
zend_try {
17571762
zend_call_destructors();

0 commit comments

Comments
 (0)