diff --git a/NEWS b/NEWS index e07d065e3852..db374320d027 100644 --- a/NEWS +++ b/NEWS @@ -74,6 +74,8 @@ PHP NEWS MariaDB PAM account). (lazerg) - Opcache: + . Fixed bug GH-23859 (Map pointer growth after OPcache resets under concurrent + traffic). (aasul) . Fixed tracing JIT returning a wrapped integer instead of a float on integer overflow after an integer was compared with a float or numeric-string literal. (Ilia Alshanetsky) diff --git a/ext/opcache/ZendAccelerator.c b/ext/opcache/ZendAccelerator.c index da3114ad60bb..60054a4c1f36 100644 --- a/ext/opcache/ZendAccelerator.c +++ b/ext/opcache/ZendAccelerator.c @@ -4844,6 +4844,8 @@ static zend_result accel_finish_startup_preload(bool in_child) bool old_reset_signals = SIGG(reset); #endif + ZCG(preloading) = true; + sapi_module.activate = NULL; sapi_module.deactivate = NULL; sapi_module.register_server_variables = NULL; @@ -4926,6 +4928,8 @@ static zend_result accel_finish_startup_preload(bool in_child) sapi_module.ub_write = orig_ub_write; sapi_module.flush = orig_flush; + ZCG(preloading) = false; + sapi_activate(); return ret; @@ -5066,6 +5070,13 @@ static zend_result accel_finish_startup(void) } static void accel_activate(void) { + /* zend_post_startup() saves the permanent boundary after preloading. + * Discard only request slots before synchronizing with SHM for this request. + * The permanent boundary is not available during preloading itself. */ + if (!ZCG(preloading)) { + zend_map_ptr_reset(); + } + if (ZCG(preloaded_internal_run_time_cache)) { memset(ZCG(preloaded_internal_run_time_cache), 0, ZCG(preloaded_internal_run_time_cache_size)); } diff --git a/ext/opcache/ZendAccelerator.h b/ext/opcache/ZendAccelerator.h index e82a1d443986..53231a4de47d 100644 --- a/ext/opcache/ZendAccelerator.h +++ b/ext/opcache/ZendAccelerator.h @@ -236,6 +236,7 @@ typedef struct _zend_accel_globals { * strings by zend_accel_pending_ce_cache_commit() once ZCSG(map_ptr_last) * covers them. NULL outside a persist window. */ HashTable *pending_ce_cache; + bool preloading; } zend_accel_globals; typedef struct _zend_string_table { diff --git a/sapi/fpm/tests/gh23859-preload.phpt b/sapi/fpm/tests/gh23859-preload.phpt new file mode 100644 index 000000000000..4cd2b1e2235e --- /dev/null +++ b/sapi/fpm/tests/gh23859-preload.phpt @@ -0,0 +1,149 @@ +--TEST-- +FPM: GH-23859 (Preloaded map pointers survive compilation and OPcache restarts) +--EXTENSIONS-- +opcache +--SKIPIF-- + +--FILE-- +getDefaultProperties() !== ['item' => 11]) { + throw new Exception('Preloaded state was not reset'); +} +PreloadedChild::$value = 23; +PreloadedChild::$items[] = 'changed'; + +/* Persist new code and load a saved preload script after filling preload slots. */ +require __DIR__ . '/gh23859-preload.compile.inc'; +require __DIR__ . '/gh23859-preload.saved.inc'; +if (PreloadedChild::$value !== 23 || PreloadedChild::$items !== ['initial', 'changed'] + || PreloadedChild::step() !== 25 || preloaded_step() !== 15 + || PreloadedChild::compute() !== 13 || RequestChild::run() !== 20 + || request_step() !== 1 || saved_step() !== 1 + || PreloadedEnum::cases() !== [PreloadedEnum::A]) { + throw new Exception('Compilation corrupted preloaded state'); +} +echo 'ok'; +EOT; + +$tester = new FPM\Tester($cfg, $code); +$preloadFile = $tester->getPrefixedFile('preload.inc'); +file_put_contents($preloadFile, <<<'EOT' +getPrefixedFile('compile.inc'), <<<'EOT' +getPrefixedFile('saved.inc'), <<<'EOT' +getPrefixedFile('reset.src.php'); +file_put_contents($resetFile, <<<'EOT' + '1', + 'opcache.file_update_protection' => '0', + 'opcache.preload' => $preloadFile, +]; +if (getenv('TEST_FPM_RUN_AS_ROOT')) { + $iniEntries['opcache.preload_user'] = 'root'; +} +$tester->start($extraArgs, iniEntries: $iniEntries); +$tester->expectLogStartNotices(); +$tester->request(address: '{{ADDR[reset]}}', scriptFilename: $resetFile)->expectBody('0'); +$tester->request(address: '{{ADDR[worker]}}')->expectBody('ok'); +$tester->request(address: '{{ADDR[worker]}}')->expectBody('ok'); + +for ($i = 1; $i <= 3; $i++) { + $tester->request(query: 'reset=1', address: '{{ADDR[reset]}}', scriptFilename: $resetFile) + ->expectBody('bool(true)'); + $tester->request(address: '{{ADDR[reset]}}', scriptFilename: $resetFile)->expectBody((string) $i); + $tester->request(address: '{{ADDR[worker]}}')->expectBody('ok'); + $tester->request(address: '{{ADDR[worker]}}')->expectBody('ok'); +} + +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/gh23859.phpt b/sapi/fpm/tests/gh23859.phpt new file mode 100644 index 000000000000..5c8ad9aacbb6 --- /dev/null +++ b/sapi/fpm/tests/gh23859.phpt @@ -0,0 +1,87 @@ +--TEST-- +FPM: GH-23859 (Map pointers grow after OPcache restarts in another worker) +--EXTENSIONS-- +opcache +zend_test +--SKIPIF-- + +--FILE-- +getPrefixedFile('reset.src.php'); +file_put_contents($resetFile, <<<'EOT' +start($extraArgs, extensions: ['zend_test'], iniEntries: [ + 'opcache.enable' => '1', + 'opcache.file_update_protection' => '0', +]); +$tester->expectLogStartNotices(); +$tester->request(address: '{{ADDR[reset]}}', scriptFilename: $resetFile)->expectBody('0'); +$before = $tester->request(address: '{{ADDR[worker]}}')->getBody(); +if (!is_numeric($before)) { + throw new Exception('Invalid map pointer count'); +} +$tester->request(address: '{{ADDR[worker]}}')->expectBody($before); + +/* Complete each restart in the other pool before the stale worker persists again. */ +for ($i = 1; $i <= 3; $i++) { + $tester->request(query: 'reset=1', address: '{{ADDR[reset]}}', scriptFilename: $resetFile) + ->expectBody('bool(true)'); + $tester->request(address: '{{ADDR[reset]}}', scriptFilename: $resetFile)->expectBody((string) $i); + $tester->request(address: '{{ADDR[worker]}}')->expectBody($before); + $tester->request(address: '{{ADDR[worker]}}')->expectBody($before); +} + +$tester->terminate(); +$tester->expectLogTerminatingNotices(); +$tester->close(); + +?> +Done +--EXPECT-- +Done +--CLEAN-- +