Skip to content

Commit

Permalink
fix(agent): revert zend_try/catch php call logic (#811)
Browse files Browse the repository at this point in the history
zend_try/zend_catch is to handle Zend Exceptions not PHP exceptions -
see
[here](https://github.com/newrelic/newrelic-php-agent/blob/320ea571a11bc469d7d8179dfe51577b54df11df/agent/php_user_instrument.c#L17-L50)
for more details. They were added in this
[commit](66eccf7)
to handle misbehaving PHP embed SAPI that threw Zend Exception when PHP
code that was called via nr_php_call threw PHP Exception. PHP CLI or CGI
SAPIs don’t throw Zend Exception when PHP code that was called threw PHP
Exception and therefore zend_try/zend_catch is effectively a dead code.
  • Loading branch information
bduranleau-nr authored Jan 31, 2024
1 parent 53d10e0 commit 4da1211
Showing 1 changed file with 15 additions and 59 deletions.
74 changes: 15 additions & 59 deletions agent/php_call.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,56 +10,6 @@

#include "Zend/zend_exceptions.h"

/*
* zend_try family of macros entail the use of setjmp and longjmp, which can cause clobbering issues with
* non-primitive local variables. Abstracting these constructs into separate functions protects from this.
*/
#if ZEND_MODULE_API_NO >= ZEND_8_2_X_API_NO
static int nr_php_call_try_catch(zend_object* object,
zend_string* method_name,
zval* retval,
zend_uint param_count,
zval* param_values) {
/*
* With PHP 8.2, functions that do not exist will cause a fatal error to
* be thrown. `zend_call_method_if_exists` will attempt to call a function and
* silently fail if it does not exist
*/
int zend_result = FAILURE;
zend_try {
zend_result = zend_call_method_if_exists(object, method_name, retval,
param_count, param_values);
}
zend_catch { zend_result = FAILURE; }
zend_end_try();
return zend_result;
}
#elif ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO \
&& ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO
static int nr_php_call_try_catch(zval* object_ptr,
zval* fname,
zval* retval,
zend_uint param_count,
zval* param_values) {
/*
* With PHP8, `call_user_function_ex` was removed and `call_user_function`
* became the recommended function.
* According to zend internals documentation:
* As of PHP 7.1.0, the function_table argument is not used and should
* always be NULL. See for more details:
* https://www.phpinternalsbook.com/php7/internal_types/functions/callables.html
*/
int zend_result = FAILURE;
zend_try {
zend_result = call_user_function(EG(function_table), object_ptr, fname,
retval, param_count, param_values);
}
zend_catch { zend_result = FAILURE; }
zend_end_try();
return zend_result;
}
#endif

zval* nr_php_call_user_func(zval* object_ptr,
const char* function_name,
zend_uint param_count,
Expand Down Expand Up @@ -116,18 +66,24 @@ zval* nr_php_call_user_func(zval* object_ptr,
}

/*
* For PHP 8+, in the case of exceptions according to:
* https://www.php.net/manual/en/function.call-user-func.php
* Callbacks registered with functions such as call_user_func() and
* call_user_func_array() will not be called if there is an uncaught exception
* thrown in a previous callback. So if we call something that causes an
* exception, it will block us from future calls that use call_user_func or
* call_user_func_array and hence the need for a try/catch block.
* With PHP 8.2, functions that do not exist will cause a fatal error to
* be thrown. `zend_call_method_if_exists` will attempt to call a function and
* silently fail if it does not exist
*/
zend_result = nr_php_call_try_catch(object, method_name, retval, param_count, param_values);
zend_result = zend_call_method_if_exists(object, method_name, retval, param_count, param_values);

#elif ZEND_MODULE_API_NO < ZEND_8_2_X_API_NO \
&& ZEND_MODULE_API_NO >= ZEND_8_0_X_API_NO
zend_result = nr_php_call_try_catch(object_ptr, fname, retval, param_count, param_values);
/*
* With PHP8, `call_user_function_ex` was removed and `call_user_function`
* became the recommended function.
* According to zend internals documentation:
* As of PHP 7.1.0, the function_table argument is not used and should
* always be NULL. See for more details:
* https://www.phpinternalsbook.com/php7/internal_types/functions/callables.html
*/
zend_result = call_user_function(EG(function_table), object_ptr, fname,
retval, param_count, param_values);

#else
zend_result = call_user_function_ex(EG(function_table), object_ptr, fname,
Expand Down

0 comments on commit 4da1211

Please sign in to comment.