Skip to content

Fix GH-9921: Loading ext in FPM config does not register module handlers #12377

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

Closed
wants to merge 4 commits into from

Conversation

bukka
Copy link
Member

@bukka bukka commented Oct 7, 2023

The collection of module handlers happened before the loading of extension so the extension loaded in FPM config was not present there. To fix it, it is necessary to re-run collection of module handlers after extensions are loaded in FPM config.

@bukka bukka changed the base branch from master to PHP-8.1 October 7, 2023 13:02
bukka added a commit to bukka/php-src that referenced this pull request Oct 7, 2023
@bukka bukka force-pushed the fpm_config_extension_module_handlers branch from 2c1a19c to 37d848c Compare October 7, 2023 13:03
@bukka
Copy link
Member Author

bukka commented Oct 7, 2023

This is based on GH-12277 as the crash needs to be fixed first. Once that gets merged I will rebase it.

Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I requested some minor changes.

It looks like loading extensions through FPM was completely broken.
Is this the last known problem?
I expect we may have more...

@bukka bukka force-pushed the fpm_config_extension_module_handlers branch from 37d848c to f6f8018 Compare November 2, 2023 14:34
@bukka
Copy link
Member Author

bukka commented Nov 2, 2023

@dstogov Requested changes done and all tests passed too.

It looks like loading extensions through FPM was completely broken.

Yeah it certainly must have not worked correctly for some extensions - especially those relaying on RINIT.

Is this the last known problem?

I'm not aware of other big issue except the ordering of extension in config which is more FPM config issue and there's also request to load zend extensions but that in the current architecture won't work properly (especially for opcache) as MINIT is in worker. Not sure about Xdebug but think Derick also mentioned some issues with that. So that's a lower priority really.

I expect we may have more...

I would expect some other issues surfacing up at some point but hopefully after this fix it should mostly work for most extensions.

Copy link
Member

@dstogov dstogov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look good

@bukka bukka closed this in a8c6c61 Nov 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants