@bukka Also added you as a reviewer since this code is quite similar to php-fpm. So you might have the most real experience with this code. 🙂

I remember fixing something related in FPM but will need to refresh my mind and double check it. I plan to also look into those argc deprecation changes and logic around that to see if anything could be improved there. I should have a slot for this in a week starting Oct 7.

@bukka Thanks. This currently breaks nightly, I'd prefer not to keep it red for so long. Maybe we can merge this for master now and backport later, once you've verified?

Maybe we can merge this for master now and backport later, once you've verified?

@iluuu1994 Sounds good to me

It took me a while to understand what is happening here and the PR description doesn't properly explain, why the field is uninitialized only in non-JIT mode. It works like this:

• zend_activate_auto_globals is being called.
• Since SERVER is not jitted, ->auto_global_callback() is being called to initialize ->armed.
• This is php_auto_globals_create_server().
• Which then calls php_register_server_variables().
• Which calls register_server_variables()
• Which calls cgi_php_import_environment_variables()
• Which calls zend_is_auto_global for ENV
• Which then depends on the uninitialized ->armed field for ENV.

An alternative fix would be initializing ENV before SERVER:

 zend_register_auto_global(zend_string_init_interned("_GET", sizeof("_GET")-1, 1), 0, php_auto_globals_create_get);
 zend_register_auto_global(zend_string_init_interned("_POST", sizeof("_POST")-1, 1), 0, php_auto_globals_create_post);
 zend_register_auto_global(zend_string_init_interned("_COOKIE", sizeof("_COOKIE")-1, 1), 0, php_auto_globals_create_cookie);
- zend_register_auto_global(ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER), PG(auto_globals_jit), php_auto_globals_create_server);
 zend_register_auto_global(ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_ENV), PG(auto_globals_jit), php_auto_globals_create_env);
+ zend_register_auto_global(ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_SERVER), PG(auto_globals_jit), php_auto_globals_create_server);
 zend_register_auto_global(ZSTR_KNOWN(ZEND_STR_AUTOGLOBAL_REQUEST), PG(auto_globals_jit), php_auto_globals_create_request);
 zend_register_auto_global(zend_string_init_interned("_FILES", sizeof("_FILES")-1, 1), 0, php_auto_globals_create_files);

This would also avoid calling php_php_import_environment_variables twice: Once to initialize $_SERVER and once to initialize $_ENV.

I went with a different fix that just makes sure armed is properly initialized. This prevents the duplicate import Tim mentioned. The commit was discussed via DM with Tim and merged directly so it makes the 8.5 RC1 cut. I'll create a new issue to backport this commit so we don't forget.