[Config] ClassExistenceResource::isFresh() throws fatal error on php 7.2.20/7.3.7 if a parent class is missing #32395
Comments
|
@teohhanhui reported that the unit tests of API platform 2.4 are failing because of a similar problem. This is what I get when I run them locally with php 7.3.7: |
|
I've managed to build a minimal reproducer. https://github.com/derrabus/symfony-32395-reproducer The problem seems to be If a class cannot be loaded because its parent is not found, a Previously, this exception could be caught and handled. Since php 7.2.20/7.3.7 however, php seems to catch exceptions thrown during class loading and turns them into a fatal error that cannot be caught. |
derrabus
changed the title
|
FYI: It is caused by https://bugs.php.net/bug.php?id=76980 which was reported because of #28748 |
|
When a class implements an interface that can not be found, PHP 7.2.20/7.3.7 throws an exception as well now. See #32396 |
No, php itself has always triggered an uncatchable fatal error, see https://3v4l.org/jogPE The trick in |
|
The new php bahavior seems fine to me. Can't we fix our code to not rely on such strange behavior? |
Correct, and that worked fine in most situation.
The error is now fatal, right? Why is it better, when the previous behavior allowed some resilience? About the strange code, sure, PHP is a runtime before a language. So: which code can we write to circumvent a fatal error? I think we should ask @nikic to consider reverting the patch on PHP. There are many situations like this one in Symfony apps that used to work seamlessly, but won't now. I hope I'm missing something and there's a workaround :) |
|
The optional services might not rely on the container optimization removing unused services but those services should only be loaded then the dependency is present. But sadly many bundles would be affected. |
That depends on what you're asking for here. I can revert this change from 7.2 and 7.3. Enforcing this is not critical for those branches and if it causes breakage in Symfony I have no problem with reverting it. For PHP 7.4 though this change is going to stay in some form. I can invest some effort to avoid throwing a fatal error, but if the request here is to restore the old behavior where you say |
This reverts commit 35353dc. This changes causes issues for Symfony, see symfony/symfony#32395. I'm reverting it from PHP 7.2 and PHP 7.3 and only leaving it in PHP 7.4.
|
Reverted from 7.2 and 7.3 via php/php-src@22ed362, this change is only in 7.4 now. |
Having thought about this for a bit, I think that covariance support in PHP 7.4 effectively makes this impossible. Due to cyclic dependencies, we sometimes have to link classes under the provision that other classes will either also successfully link or else be unusable. This means that we can't just unregister the class stub if a parent/interface is not found, because then it would be possible to register a class with different methods under the same name and thus violate variance assumptions. The only thing we could do is leave behind a dead class stub, such that the class can neither be used, nor a class using the same name declared, which seems like a pretty bad idea. |
|
@nikic Can we find another way to make autoloading failures recoverable somehow? To stay in your example, what the |
|
@derrabus I don't think so. Let me try to give a specific example of the problem I have in mind, though it's somewhat convoluted: // A.php
class A {
public function foo($x): B {}
}
// B.php
class B extends A {
public function foo($x): C {}
}
// C.php
class C extends B implements DoesNotExist {
}
// main.php
new C;What happens now is the following:
After these steps have happened ... what can we do now? We can't fully register the class I don't really see a solution here that would allow us to gracefully recover while performing inheritance :( |
|
I think the correct thing to do is to remove both |
|
@teohhanhui The class // B.php
class B extends A {
public function foo($x): C {}
}
$b = new B;Of course, we can't drop the class |
|
Does it make sense then to only fail if class |
|
Thank you for the detailed explanation, @nikic! Return type contravariance really complicates this. 😕 |
|
Hey @derrabus , glad that YOU are on it ;) !! We're deploying on Heroku and it's using PHP 7.2.20 / PHP 7.3.7. There's no simple option to change that. The "-latest" official docker hub release for PHP also stays at .19 / .6. Effect: we currently cannot deploy anything. Is there a chance that this is rolled back very soon (@nikic) or fixed by any change in Symfony? I'm a little shocked that this "breaking change" in PHP simply went upstream since Symfony+Doctrine is for sure not an unusual use / test case in the PHP world... 😱 |
|
@elmariachi111 I don't know how Heroku handles a situation like this, but there has to be a way to deploy to an older php release. Have you contacted their support yet? If there really isn't another way, my suggestion would be:
That'll bloat your vendors folder, but you should be able to run your application again. Please remember to revert that change once php 7.2.21 and 7.3.8 are out. |
|
@mmarton too tricky, just rolled back already |
|
For Ubuntu and Debian users you can now update your php binaries, oerdnj fix it : |
|
Filed a feature request in PHP: |
|
Remove Entity from exclude list in service.yaml fix for me. |
|
https://github.com/php/php-src/releases |
|
released two days ago, and yet still now showing on the official PHP downloads page: https://www.php.net/downloads.php hopefully the various distros will distribute the updates soon. |
It's tagged but not released just yet. Releases tend to happen on Thursdays if I'm not mistaken, so likely tomorrow :) |
|
PHP 7.2.21 doesn't include 78351 as a fixed issue at at https://www.php.net/ChangeLog-7.php#7.2.21, and https://bugs.php.net/bug.php?id=78351 is still listed as open - is there any way of verifying it's been fixed in there? |
|
It contains the revert of the fix that caused the issue. |
|
Excellent, thanks @mmarton |
…g parent classes (nicolas-grekas) This PR was merged into the 3.4 branch. Discussion ---------- Skip tests that fatal-error on PHP 7.4 because of missing parent classes | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #32844 | License | MIT | Doc PR | - See #32395 and https://bugs.php.net/78351 for more background. This skips the affected tests with a warning, which means tests won't pass so we won't forget about them. Commits ------- c2c7ba8 Skip tests that fatal-error on PHP 7.4 because of missing parent classes
|
looks like the Remi repo is distributing the new releases, just now waiting on Ondrej for the Debian/Ubuntu users. |
|
@arderyp Ondřej provides patched packages since Jul 25 as per oerdnj/deb.sury.org#1208 (comment) |
|
@discordier, right, patched, but not latest releases |
|
Closing as this has been fixed in PHP. |
and others
Symfony version(s) affected: 3.4.29, 4.2.10, 4.3.2, 4.4-dev, 5.0-dev
Description
After upgrading to php 7.2.20 or 7.3.7,
AutowirePassstarts to throwReflectionExceptions on cache warmup on installations with DoctrineBundle but without Twig and the Form and Validator components. This has been reported to the DoctrineBundle repository, but to me it rather looks like a DI issue.How to reproduce
The text was updated successfully, but these errors were encountered: