Skip to content
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.

Sign up for GitHub

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

Jump to bottom

opcache_compile_file() fails with Cannot redeclare function error when compiling multiple files with the same function #16668

Open
joec4i opened this issue Nov 1, 2024 · 3 comments · May be fixed by #16862
Open

opcache_compile_file() fails with Cannot redeclare function error when compiling multiple files with the same function #16668

joec4i opened this issue Nov 1, 2024 · 3 comments · May be fixed by #16862
Labels
Bug Extension: opcache Status: Verified

Comments

@joec4i
Copy link
Contributor

joec4i commented Nov 1, 2024 •
edited
Loading

Description

The following code:

compile-test.php

<?php
var_dump(opcache_compile_file(__DIR__ . '/releases/v1/functions.php'));

// expecting `bool(false)` since opcache_compile_file() shouldn't have executed the compiled file.
var_dump(in_array('foo', get_defined_functions()['user']));

// expecting successful compilation: `bool(true)`
var_dump(opcache_compile_file(__DIR__ . '/releases/v2/functions.php'));

releases/v1/functions.php

<?php
function foo(): string {
    return 'foo_v1';
}

releases/v2/functions.php

<?php
function foo(): string {
    return 'foo_v2';
}

Resulted in this output:

bool(true)
bool(true)
PHP Fatal error:  Cannot redeclare foo() (previously declared in /private/tmp/opcache/releases/v1/functions.php:4) in /private/tmp/opcache/releases/v2/functions.php on line 4

Fatal error: Cannot redeclare foo() (previously declared in /private/tmp/opcache/releases/v1/functions.php:4) in /private/tmp/opcache/releases/v2/functions.php on line 4
PHP Warning:  Zend OPcache could not compile file /private/tmp/opcache/releases/v2/functions.php in /private/tmp/opcache/compile-test.php on line 4

Warning: Zend OPcache could not compile file /private/tmp/opcache/releases/v2/functions.php in /private/tmp/opcache/compile-test.php on line 4
bool(false)

But I expected this output instead:

bool(true)
bool(false)
bool(true)

Additional Context:
The issue happens on both cli and php-fpm. It seems to be similar to https://bugs.php.net/bug.php?id=66066 . While the use case in bug 66066 might be less common in the real world and probably should be avoided, it's a perfectly valid use case to run multiple releases with similar files under the same php-fpm instance.

Workaround
Our current workaround is to compile different releases with different requests, i.e. PHP will happily process the following logics in two different requests.

compile-v1.php

<?php
var_dump(opcache_compile_file(__DIR__ . '/releases/v1/functions.php'));

compile-v2.php

<?php
var_dump(opcache_compile_file(__DIR__ . '/releases/v2/functions.php'));

opcache_get_status()['scripts'] shows that the two files have been compiled successfully.

    [/var/www/html/releases/v2/functions.php] => Array
        (
            [full_path] => /var/www/html/releases/v2/functions.php
            [hits] => 0
            [memory_consumption] => 992
            [last_used] => Fri Nov  1 11:32:01 2024
            [last_used_timestamp] => 1730460721
            [timestamp] => 1730458160
            [revalidate] => 1730460723
        )

    [/var/www/html/releases/v1/functions.php] => Array
        (
            [full_path] => /var/www/html/releases/v1/functions.php
            [hits] => 5
            [memory_consumption] => 992
            [last_used] => Fri Nov  1 11:31:58 2024
            [last_used_timestamp] => 1730460718
            [timestamp] => 1730458130
            [revalidate] => 1730460720
        )

PHP Version

PHP 8.3.13

Operating System

No response
@joec4i
Copy link
Contributor Author

joec4i commented Nov 1, 2024

I have put my test setup in https://github.com/joec4i/opcache-compile-tests . Thanks!

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Nov 19, 2024
@iluuu1994
Avoid registering top functions with opcache_compile_file()
72082ad 
Previously, this only worked for classes. It worked by preventing early binding,
and then declaring the class at runtime. Instead of doing that, we now avoid
calling zend_accel_load_script() completely, which prevents the functions from
being added to the function table.

Fixes phpGH-16668
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Nov 19, 2024
@iluuu1994
Avoid registering top functions with opcache_compile_file()
f808644 
Previously, this only worked for classes. It worked by preventing early binding,
and then declaring the class at runtime. Instead of doing that, we now avoid
calling zend_accel_load_script() completely, which prevents the functions from
being added to the function table.

Fixes phpGH-16668
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Nov 19, 2024
@iluuu1994
Avoid registering top functions with opcache_compile_file()
43026a5 
Previously, this only worked for classes. It worked by preventing early binding,
and then declaring the class at runtime. Instead of doing that, we now avoid
calling zend_accel_load_script() completely, which prevents the functions from
being added to the function table.

Fixes phpGH-16668
@iluuu1994 iluuu1994 linked a pull request Nov 19, 2024 that will close this issue
Open
@iluuu1994
Copy link
Member

Hi @joec4i, thanks for your report, and sorry for the delay. I agree that at least the documentation is misleading.

https://www.php.net/manual/en/function.opcache-compile-file.php

This function compiles a PHP script and adds it to the opcode cache without executing it. This can be used to prime the cache after a Web server restart by pre-caching files that will be included in later requests.

The fix1 is breaking though, so we may not fix it in any supported versions. It should also be discussed on the mailing list.

Footnotes

  1. https://github.com/php/php-src/pull/16862 ↩

@danog
Copy link
Contributor

danog commented Nov 21, 2024 •
edited
Loading

I believe the removal of function caching is a bit of an overreaction: it was always known, to those who used it, that preloading will cache functions, and requires an include guard to avoid function redeclaration errors, and in fact it is a very useful feature.

There is nothing wrong with this behaviour, and it does what you expect it to do.

I believe I was slightly confused by a different issue I previously dealt with i.e. amphp/amp#321

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Extension: opcache Status: Verified
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Avoid registering top functions with opcache_compile_file() iluuu1994/php-src
3 participants
@joec4i @iluuu1994 @danog