-
Notifications
You must be signed in to change notification settings - Fork 160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix #2030: Segmentation fault with autoloaders bailing out #2037
Conversation
We fix this issue by ensuring that the opline is always connected to its proper execute_data and popping newer entries if they aren't matching. Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
--TEST-- | ||
Assert bailouts are gracefully handled within class autoloading | ||
--SKIPIF-- | ||
<?php if (PHP_VERSION_ID >= 70300 && PHP_VERSION_ID < 70400) die('skip: Bailing out in autoloaders is fundamentally broken in PHP 7.3'); ?> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question (non-blocking): What would be the expected behaviour in PHP 7.3, segfault?
Do we document such things somewhere, just in case another customer experiences this problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this segfaults. It's plain and simple a bug in PHP 7.3 itself.
For now I'm not aware of such documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved.
} | ||
if (prev_exception_hook) { | ||
prev_exception_hook(ex); | ||
} | ||
} | ||
|
||
void zai_interceptor_reset_resolver() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nit.
void zai_interceptor_reset_resolver() { | |
void zai_interceptor_reset_resolver(void) { |
} | ||
if (prev_exception_hook) { | ||
prev_exception_hook(ex); | ||
} | ||
} | ||
|
||
void zai_interceptor_reset_resolver() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And matching nit.
void zai_interceptor_reset_resolver() { | |
void zai_interceptor_reset_resolver(void) { |
Signed-off-by: Bob Weinand <bob.weinand@datadoghq.com>
Description
We fix this issue by ensuring that the opline is always connected to its proper execute_data and popping newer entries if they aren't matching.
Note for the reviewer: the PHP 7 and PHP 8 changes are identical.
Readiness checklist
Reviewer checklist