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::publishinstrumentation, 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