opentelemetry-php: [opentelemetry-php-instrumentation] - Segfault when increasing arguments in pre hook for native functions/methods
PHP 8.2.7 with latest opentelemetry 1.0.0beta6.
Steps to reproduce Given this code where we modify the arguments will crash with a segfault.
<?php
OpenTelemetry\Instrumentation\hook(
null,
'array_slice',
pre: function(null $instance, array $params) {
return [$params[0], $params[1], 1];
},
post: function(null $instance, array $params, $returnValue) {
}
);
var_dump(array_slice([1,2,3], 1));
This will not crash:
null,
'array_slice',
pre: function(null $instance, array $params) {
- return [$params[0], $params[1], 1];
+ return [$params[0], $params[1]];
},
post: function(null $instance, array $params, $returnValue) {
}
This will also not crash:
'array_slice',
pre: function(null $instance, array $params) {
return [$params[0], $params[1], 1];
- },
- post: function(null $instance, array $params, $returnValue) {
}
);
It will also not crash if the function is a userland function, so it only crashes if the function is defined in a PHP extension (core or PECL). I originally discovered the crash while working on instrumentation for PECL amqp, specifically when hooking into AMQPExchange::publish
and expanding the args there.
About this issue
- Original URL
- State: closed
- Created a year ago
- Comments: 19 (3 by maintainers)
@lstrojny I’ve created a new issue for tracking this as an enhancement request rather than a bug: https://github.com/open-telemetry/opentelemetry-php/issues/1223
Since the main concern with this issue (based on the title) was the crash and that is now fixed, it makes sense to handle the feature request in a separate issue.
You can add a short comment there that one desired use case for that feature would specifically be
AMQPExchange::publish
instrumentation, so it would be clear that the issue is actually relevant right now.I spent some time on this today. No fixes yet, but it seems to be related to garbage collection: the exception is in php shutdown. Whilst playing around, I also came across another failure scenario: we can’t return $params from a pre callback. The two may be related, as they’re both to do with ref counting for garbage collection, I think. open-telemetry/opentelemetry-php-instrumentation#71