Print a custom error message on platforms with ext/json as shared extension

[bjori]

Before:

$ php -n -dextension=modules/mongodb.so -r 'echo phpversion("mongodb"), "\n";' PHP Warning: PHP Startup: Unable to load dynamic library 'modules/mongodb.so' - modules/mongodb.so: undefined symbol: php_json_serializable_ce in Unknown on line 0 

After:

$ php -n -dextension=modules/mongodb.so -r 'echo phpversion("mongodb"), "\n";' Please make sure json.so is loaded before mongodb.so PHP Warning: Cannot load module 'mongodb' because required module 'json' is not loaded in Unknown on line 0 

[bjori]

The extra warning there is of course not printed when things go fine:

$ php -n -dextension=modules/mongodb.so -r 'echo phpversion("mongodb"), "\n";' 1.2.1-dev 

[bjori]

Related issues:

• Driver on Debian 8 #478
• Unable to load dynamic library after upgrade to 1.2.0 "undefined symbol: php_json_serializable_ce" #477
• Undefined Symbol php_json_serializable_ce in Unknown on Line 0 (1.2.0 Release) #475

[derickr]

I left a few comments, but, in general, I am not sure whether we should do this.

It is really a distribution's problem. Their binaries mess up PHP's normal extension API, so they should also provide the solutions to fix this, or provide the extensions as binaries.

[derickr]

This does not seem to be called from anywhere?

[bjori]

Its called automatically during dlopen

[derickr]

That's a problem then, as it would mean that PHP's normal loading reordering will no longer work. Basically, making the off case work (where distributions mangle PHP's original itents), but not the standard case where PHP itself manages the loading/symbol fetching order (the patch actively prevents this from working now).

[jmikola]

Does __attribute__((constructor)) result in this preempting PHP's own extension dependency resolution based on the list in our zend_module_entry?

[jmikola]

@derickr: How does this affect PHP's ordering if we're still keeping the dependency in our zend_module_dep list? @bjori linked me to zend_startup_module_ex(), which just appears to check that dependencies are already loaded at the time our extension is loaded.

If we're loaded via dlopen() before json.so and this triggers, hasn't PHP already failed to ensure our json.so dependency?

[derickr]

PHP's mechanism is as follows:

• dlopen() all extensions in their loading order (order as in .ini file). This works normally, as LTDL_LAZY is being used and no symbols are being resolved yet.
• create dependency graph
• go initialize extensions in the right order as to not have undefined symbols

The new constructor is run on dlopen(), and hence, breaks the original mechanism of being to open extensions in a random order.

[derickr]

Some OSses prefix the symbols with a _, and no check for that is done here.

See: https://github.com/php/php-src/blob/master/ext/standard/dl.c#L142

[bjori]

I was under the impression the macro abstracted it. Looking it up, I guess not

[remicollet]

I have doubt about this working if php_json_serializable_ce is not publicly exported.

[bjori]

this isn't actually needed. as long as the references to the symbol are removed and it rather dynamically loaded, like the rest of the patch.
This function is only for the purpose of augmenting the error message from php if they want to give direct instructions

[derickr]

zend_string_init is new in PHP 7.0, so this needs an #ifdef.

[bjori]

This PR actually does two things.

1. Removes the need for that symbol, so PHPs error message is properly displayed.
2. Augments the PHP error message with custom error to guide the user

I consider this to be significant improvement to the user experience and even it is not the mongodb drivers fault, we can still improve the experience for the users

[derickr]

That's a problem then, as it would mean that PHP's normal loading reordering will no longer work. Basically, making the off case work (where distributions mangle PHP's original itents), but not the standard case where PHP itself manages the loading/symbol fetching order (the patch actively prevents this from working now).

[jmikola]

See #500.