Skip to content
This repository was archived by the owner on Oct 3, 2023. It is now read-only.

Commit a34e27b

Browse files
chingor13chingor13
authored
Extension: Fix timing memory management for callback arguments (#184)
* Add basic test for failing argument handling on php 7.1 Fix method test Fix method test * Add tests for array arguments * Remove debug statement * opencensus_trace_begin does not need to execute callbacks * Copy args before executing the original function, then run callback with args * Refactor execute handlers with guards * Fix memory leaks
1 parent 2e3e731 commit a34e27b

File tree

5 files changed

+237
-62
lines changed

5 files changed

+237
-62
lines changed

‎ext/opencensus_trace.c‎

Lines changed: 93 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -302,35 +302,41 @@ PHP_FUNCTION(opencensus_trace_add_message_event)
302302
RETURN_FALSE;
303303
}
304304

305-
/**
306-
* Call the provided callback with the provided parameters to the traced
307-
* function. The callback must return an array or an E_WARNING is raised.
308-
*/
309-
static int opencensus_trace_call_user_function_callback(zend_execute_data *execute_data, opencensus_trace_span_t *span, zval *callback, zval *callback_result TSRMLS_DC)
305+
static void opencensus_copy_args(zend_execute_data *execute_data, zval **args, int *ret_num_args)
310306
{
311-
int i, num_args = EX_NUM_ARGS(), has_scope = 0;
312-
zval *args = emalloc((num_args + 1) * sizeof(zval));
307+
int i, num_args = ZEND_CALL_NUM_ARGS(execute_data), has_scope = 0;
308+
zval *arguments = emalloc((num_args + 1) * sizeof(zval));
309+
*args = arguments;
313310

314311
if (getThis() != NULL) {
315312
has_scope = 1;
316-
ZVAL_COPY(&args[0], getThis());
313+
ZVAL_COPY(&arguments[0], getThis());
317314
}
318315

319316
for (i = 0; i < num_args; i++) {
320-
ZVAL_COPY(&args[i + has_scope], EX_VAR_NUM(i));
317+
ZVAL_COPY(&arguments[i + has_scope], ZEND_CALL_VAR_NUM(execute_data, i));
321318
}
319+
*ret_num_args = num_args + has_scope;
320+
}
322321

323-
if (call_user_function_ex(EG(function_table), NULL, callback, callback_result, num_args + has_scope, args, 0, NULL) != SUCCESS) {
324-
for (i = 0; i < num_args + has_scope; i++) {
325-
ZVAL_DESTRUCTOR(&args[i]);
326-
}
327-
efree(args);
328-
return FAILURE;
329-
}
330-
for (i = 0; i < num_args + has_scope; i++) {
322+
static void opencensus_free_args(zval *args, int num_args)
323+
{
324+
int i;
325+
for (i = 0; i < num_args; i++) {
331326
ZVAL_DESTRUCTOR(&args[i]);
332327
}
333328
efree(args);
329+
}
330+
331+
/**
332+
* Call the provided callback with the provided parameters to the traced
333+
* function. The callback must return an array or an E_WARNING is raised.
334+
*/
335+
static int opencensus_trace_call_user_function_callback(zval *args, int num_args, zend_execute_data *execute_data, opencensus_trace_span_t *span, zval *callback, zval *callback_result TSRMLS_DC)
336+
{
337+
if (call_user_function_ex(EG(function_table), NULL, callback, callback_result, num_args, args, 0, NULL) != SUCCESS) {
338+
return FAILURE;
339+
}
334340

335341
if (EG(exception) != NULL) {
336342
php_error_docref(NULL, E_WARNING, "Exception in trace callback");
@@ -347,29 +353,6 @@ static int opencensus_trace_call_user_function_callback(zend_execute_data *execu
347353
return SUCCESS;
348354
}
349355

350-
/**
351-
* Handle the callback for the traced method depending on the type
352-
* - if the zval is an associative array, then assume it's the trace span initialization
353-
* options
354-
* - if the zval is an array that looks like a callable, then assume it's a callable
355-
* - if the zval is a Closure, then execute the closure and take the results as
356-
* the trace span initialization options
357-
*/
358-
static void opencensus_trace_execute_callback(opencensus_trace_span_t *span, zend_execute_data *execute_data, zval *span_options TSRMLS_DC)
359-
{
360-
zend_string *callback_name = NULL;
361-
if (zend_is_callable(span_options, 0, &callback_name)) {
362-
zval callback_result;
363-
if (opencensus_trace_call_user_function_callback(execute_data, span, span_options, &callback_result TSRMLS_CC) == SUCCESS) {
364-
opencensus_trace_span_apply_span_options(span, &callback_result);
365-
}
366-
ZVAL_DESTRUCTOR(&callback_result);
367-
} else if (Z_TYPE_P(span_options) == IS_ARRAY) {
368-
opencensus_trace_span_apply_span_options(span, span_options);
369-
}
370-
zend_string_release(callback_name);
371-
}
372-
373356
/**
374357
* Force the random span id to be positive. php_mt_rand() generates 32 bits
375358
* of randomness. On 32-bit systems, we must cast to an unsigned int before
@@ -491,12 +474,12 @@ PHP_FUNCTION(opencensus_trace_begin)
491474
if (span_options == NULL) {
492475
array_init(&default_span_options);
493476
span = opencensus_trace_begin(function_name, execute_data, NULL TSRMLS_CC);
494-
opencensus_trace_execute_callback(span, execute_data, &default_span_options TSRMLS_CC);
477+
opencensus_trace_span_apply_span_options(span, &default_span_options);
495478
ZVAL_DESTRUCTOR(&default_span_options);
496479
} else {
497480
span_id = span_id_from_options(Z_ARR_P(span_options));
498481
span = opencensus_trace_begin(function_name, execute_data, span_id TSRMLS_CC);
499-
opencensus_trace_execute_callback(span, execute_data, span_options TSRMLS_CC);
482+
opencensus_trace_span_apply_span_options(span, span_options);
500483
}
501484

502485
RETURN_TRUE;
@@ -614,22 +597,46 @@ void opencensus_trace_execute_ex (zend_execute_data *execute_data TSRMLS_DC) {
614597
);
615598
zval *trace_handler;
616599
opencensus_trace_span_t *span;
600+
zend_string *callback_name = NULL;
601+
602+
/* Some functions have no names - just execute them */
603+
if (function_name == NULL) {
604+
opencensus_original_zend_execute_ex(execute_data TSRMLS_CC);
605+
return;
606+
}
617607

618-
if (function_name) {
619-
trace_handler = zend_hash_find(OPENCENSUS_TRACE_G(user_traced_functions), function_name);
608+
trace_handler = zend_hash_find(OPENCENSUS_TRACE_G(user_traced_functions), function_name);
620609

621-
if (trace_handler != NULL) {
622-
span = opencensus_trace_begin(function_name, execute_data, NULL TSRMLS_CC);
623-
opencensus_original_zend_execute_ex(execute_data TSRMLS_CC);
624-
opencensus_trace_execute_callback(span, execute_data, trace_handler TSRMLS_CC);
625-
opencensus_trace_finish();
626-
} else {
627-
opencensus_original_zend_execute_ex(execute_data TSRMLS_CC);
628-
}
610+
/* Function is not registered for execution - continue normal execution */
611+
if (trace_handler == NULL) {
612+
opencensus_original_zend_execute_ex(execute_data TSRMLS_CC);
629613
zend_string_release(function_name);
614+
return;
615+
}
616+
617+
span = opencensus_trace_begin(function_name, execute_data, NULL TSRMLS_CC);
618+
zend_string_release(function_name);
619+
620+
if (zend_is_callable(trace_handler, 0, &callback_name)) {
621+
/* Registered handler is callable - execute the callback */
622+
zval callback_result, *args;
623+
int num_args;
624+
opencensus_copy_args(execute_data, &args, &num_args);
625+
opencensus_original_zend_execute_ex(execute_data TSRMLS_CC);
626+
if (opencensus_trace_call_user_function_callback(args, num_args, execute_data, span, trace_handler, &callback_result TSRMLS_CC) == SUCCESS) {
627+
opencensus_trace_span_apply_span_options(span, &callback_result);
628+
}
629+
opencensus_free_args(args, num_args);
630+
ZVAL_DESTRUCTOR(&callback_result);
630631
} else {
632+
/* Registered handler is span options array */
631633
opencensus_original_zend_execute_ex(execute_data TSRMLS_CC);
634+
if (Z_TYPE_P(trace_handler) == IS_ARRAY) {
635+
opencensus_trace_span_apply_span_options(span, trace_handler);
636+
}
632637
}
638+
zend_string_release(callback_name);
639+
opencensus_trace_finish();
633640
}
634641

635642
/**
@@ -657,22 +664,46 @@ void opencensus_trace_execute_internal(INTERNAL_FUNCTION_PARAMETERS)
657664
);
658665
zval *trace_handler;
659666
opencensus_trace_span_t *span;
667+
zend_string *callback_name = NULL;
660668

661-
if (function_name) {
662-
trace_handler = zend_hash_find(OPENCENSUS_TRACE_G(user_traced_functions), function_name);
669+
/* Some functions have no names - just execute them */
670+
if (function_name == NULL) {
671+
resume_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU);
672+
return;
673+
}
663674

664-
if (trace_handler) {
665-
span = opencensus_trace_begin(function_name, execute_data, NULL TSRMLS_CC);
666-
resume_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU);
667-
opencensus_trace_execute_callback(span, execute_data, trace_handler TSRMLS_CC);
668-
opencensus_trace_finish();
669-
} else {
670-
resume_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU);
671-
}
675+
trace_handler = zend_hash_find(OPENCENSUS_TRACE_G(user_traced_functions), function_name);
676+
677+
/* Function is not registered for execution - continue normal execution */
678+
if (trace_handler == NULL) {
679+
resume_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU);
672680
zend_string_release(function_name);
681+
return;
682+
}
683+
684+
span = opencensus_trace_begin(function_name, execute_data, NULL TSRMLS_CC);
685+
zend_string_release(function_name);
686+
687+
if (zend_is_callable(trace_handler, 0, &callback_name)) {
688+
/* Registered handler is callable - execute the callback */
689+
zval callback_result, *args;
690+
int num_args;
691+
opencensus_copy_args(execute_data, &args, &num_args);
692+
resume_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU);
693+
if (opencensus_trace_call_user_function_callback(args, num_args, execute_data, span, trace_handler, &callback_result TSRMLS_CC) == SUCCESS) {
694+
opencensus_trace_span_apply_span_options(span, &callback_result);
695+
}
696+
opencensus_free_args(args, num_args);
697+
ZVAL_DESTRUCTOR(&callback_result);
673698
} else {
699+
/* Registered handler is span options array */
674700
resume_execute_internal(INTERNAL_FUNCTION_PARAM_PASSTHRU);
701+
if (Z_TYPE_P(trace_handler) == IS_ARRAY) {
702+
opencensus_trace_span_apply_span_options(span, trace_handler);
703+
}
675704
}
705+
zend_string_release(callback_name);
706+
opencensus_trace_finish();
676707
}
677708

678709
/**

‎ext/tests/function_callback_arguments_array.phpt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
OpenCensus Trace: Function callback closure that reads array arguments
3+
--FILE--
4+
<?php
5+
6+
function func1($n)
7+
{
8+
return $n;
9+
}
10+
11+
opencensus_trace_function('func1', function ($n) {
12+
return [
13+
'attributes' => [
14+
'n' => count($n)
15+
]
16+
];
17+
});
18+
19+
$result = func1([1,2,3]);
20+
echo "Result: " . implode(',', $result) . PHP_EOL;
21+
22+
$spans = opencensus_trace_list();
23+
echo "Number of spans: " . count($spans) . "\n";
24+
$span = $spans[0];
25+
26+
print_r($span->attributes());
27+
?>
28+
--EXPECT--
29+
Result: 1,2,3
30+
Number of spans: 1
31+
Array
32+
(
33+
[n] => 3
34+
)

‎ext/tests/function_callback_arguments_string.phpt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
OpenCensus Trace: Customize the trace span options for a function with a callback closure that reads string arguments
3+
--FILE--
4+
<?php
5+
6+
function func1($n)
7+
{
8+
return $n;
9+
}
10+
11+
opencensus_trace_function('func1', function ($n) {
12+
return [
13+
'attributes' => [
14+
'n' => $n
15+
]
16+
];
17+
});
18+
19+
$result = func1('hello');
20+
echo "Result: $result" . PHP_EOL;
21+
22+
$spans = opencensus_trace_list();
23+
echo "Number of spans: " . count($spans) . "\n";
24+
$span = $spans[0];
25+
26+
print_r($span->attributes());
27+
?>
28+
--EXPECT--
29+
Result: hello
30+
Number of spans: 1
31+
Array
32+
(
33+
[n] => hello
34+
)

‎ext/tests/method_callback_arguments_array.phpt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
OpenCensus Trace: Method callback closure that reads array arguments
3+
--FILE--
4+
<?php
5+
6+
class TestClass
7+
{
8+
public function func1($n)
9+
{
10+
return $n;
11+
}
12+
}
13+
14+
opencensus_trace_method(TestClass::class, 'func1', function ($obj, $n) {
15+
return [
16+
'attributes' => [
17+
'n' => count($n)
18+
]
19+
];
20+
});
21+
22+
$obj = new TestClass();
23+
$result = $obj->func1([1,2,3]);
24+
echo "Result: " . implode(',', $result) . PHP_EOL;
25+
26+
$spans = opencensus_trace_list();
27+
echo "Number of spans: " . count($spans) . "\n";
28+
$span = $spans[0];
29+
30+
print_r($span->attributes());
31+
?>
32+
--EXPECT--
33+
Result: 1,2,3
34+
Number of spans: 1
35+
Array
36+
(
37+
[n] => 3
38+
)

‎ext/tests/method_callback_arguments_string.phpt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
OpenCensus Trace: Customize the trace span options for a function with a callback closure that reads string arguments
3+
--FILE--
4+
<?php
5+
6+
class TestClass
7+
{
8+
public function func1($n)
9+
{
10+
return $n;
11+
}
12+
}
13+
14+
opencensus_trace_method(TestClass::class, 'func1', function ($obj, $n) {
15+
return [
16+
'attributes' => [
17+
'n' => $n
18+
]
19+
];
20+
});
21+
22+
$obj = new TestClass();
23+
$result = $obj->func1('hello');
24+
echo "Result: $result" . PHP_EOL;
25+
26+
$spans = opencensus_trace_list();
27+
echo "Number of spans: " . count($spans) . "\n";
28+
$span = $spans[0];
29+
30+
print_r($span->attributes());
31+
?>
32+
--EXPECT--
33+
Result: hello
34+
Number of spans: 1
35+
Array
36+
(
37+
[n] => hello
38+
)

0 commit comments

Comments
 (0)