Repository navigation
Add per-extension child initialization hooks - #24647
Conversation
|
The latest revision advances each child RNG state by a PID-derived distance without reseeding or changing the parent state. The reviewer confirmed this should work; all CI checks are green. |
arnaud-lb
left a comment
There was a problem hiding this comment.
Life cycles look ok: the seed is initialized once per thread from a CSPRNG, and advanced after a fork.
Please add new macros to declare and refer to the child init function, as we do for other hooks.
9be2801 to
3d46385
Compare
|
Addressed the review by adding macros to declare and reference child-init hooks. Fixed CI build failures by replacing obsolete atomic-store calls in zend_test; checks are rerunning. |
|
Fixed the CI build failures by restoring the missing generated arginfo entries for both zend_test functions. The CI checks are running again. |
17bab85 to
ec720cd
Compare
ec720cd to
ce8ba69
Compare
3b86d34 to
3df5b71
Compare
|
About the
/* We can even omit the zero ones here: */
#define STANDARD_MODULE_PROPERTIES_EX \
.module_started = 0, \
.type = 0, \
.handle = NULL, \
.module_number = 0, \
.build_id = ZEND_MODULE_BUILD_ID
struct _zend_module_entry {
// ...
zend_result (*post_deactivate_func)(void);
void (*child_init_func)(void), // the new one
int module_started;
unsigned char type;
void *handle;
int module_number;
const char *build_id;
};
zend_module_entry zend_test_module_entry = {
STANDARD_MODULE_HEADER,
"zend_test",
ext_functions,
PHP_MINIT(zend_test),
PHP_MSHUTDOWN(zend_test),
PHP_RINIT(zend_test),
PHP_RSHUTDOWN(zend_test),
PHP_MINFO(zend_test),
PHP_ZEND_TEST_VERSION,
PHP_MODULE_GLOBALS(zend_test),
PHP_GINIT(zend_test),
PHP_GSHUTDOWN(zend_test),
NULL,
.child_init_func = PHP_CINIT(zend_test),
STANDARD_MODULE_PROPERTIES_EX,
};The mix between named and unnamed is a bit ugly, so extensions that use |
I'm a big fan of named initializers. I'm already annoyed when I want to add additional hooks after the fact and need to count which |
|
@arnaud-lb @TimWolla I updated to use named initializers. The field name makes the callback’s purpose clear, so no comment is needed. |
|
Thank you @kairosci. Please resolve #24647 (comment) and then we are good to merge. Also please rebase and re-run |
f0853ab to
e5c2efb
Compare
Fixes phpGH-22616. Add zend_module_entry.child_init_func and invoke it from php_child_init() for started modules in dependency order after core child initialization. Expose PHP_CINIT/PHP_CINIT_FUNCTION macros and use named initializers for the touched module properties. In ext/session, seed GC randomness during GINIT and advance the PRNG state in the child init hook so forked children do not share the same GC stream. Keep session GC state in TLS globals. Add zend_test coverage, a session fork regression test, and an FPM worker recycling test. Regenerate zend_test arginfo.
e5c2efb to
3d5da1a
Compare
|
@TimWolla @arnaud-lb I think I've addressed all the reviews and conflicts. |
|
Thank you! |
Fixes #22616 by adding per-extension child initialization hooks and invalidating inherited session GC randomness after forks. Validated with debug NTS and ZTS builds, shared extensions, nested forks, FPM worker recycling, and unavailable entropy sources.