-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Redis TagAwareCacheInterface stopped working after upgrade to 5.4.2 #44954
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.
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
Comments
Maybe related to #44682 which made adjustments to redis caching with tags. I'll try and take a look later today. |
Hey @madmortigan1, I tried to reproduce best I could based on your description but couldn't duplicate your problem. I'll need a bit more to go on. Here's my reproducer and the reproduction commit. |
Hi kbond, docker-compose.yamlredis: I did not mention earlier, I will add that I added the following lines to the services.yaml for handling session ` services.yaml
` I saw you worked with predis, I am not familer with it. I did not add any additional redis packages. What was really buffleing is that I could not debug the issue or catch any exception to try and catch the issue, I checked the redis container to see if the keys were saved anyway but that was not the case. I did try to run bin/console cache:clear which worked without problem, so I am guessing it could connect to the redis container. Hope this helps to find the issue. I am more than willing to share with you online if it helps to find the issue. |
@madmortigan1 could you please put the reproducer in a git repository on github that we could clone and launch easily? |
Yes, I will try to reproduce it and upload it for your convenience. |
Hi,
composer install inside the docker and go to http://localhost:851/default (or any link you changed to) Thank you, |
@madmortigan1 where is the repo located? |
@nicolas-grekas Yes, I'll say it was funny on my part. Here is the link: |
@madmortigan1, can you make it public? |
@kbond I prefare not, but it was done. |
Thanks for the reproducer. This looks like a crash of PHP. --- a/src/Symfony/Component/Cache/Traits/ContractsTrait.php
+++ b/src/Symfony/Component/Cache/Traits/ContractsTrait.php
@@ -42,7 +42,7 @@ trait ContractsTrait
public function setCallbackWrapper(?callable $callbackWrapper): callable
{
if (!isset($this->callbackWrapper)) {
- $this->callbackWrapper = \Closure::fromCallable([LockRegistry::class, 'compute']);
+ $this->callbackWrapper = [LockRegistry::class, 'compute']; Please submit a bug report to php-src. Can you please also try this with symfony/cache 6.0? Is the issue the same? |
I patched Symfony in 256ce7f but please follow up this on php-src and link back here when doing so. |
There are issues with latest php 8.1 container where I am waitting for a critical patch to continue the testing. Apperently they have problems with sockets and the container is not built correctly. I guess I will have to wait a little longer to continue with this issue and stay with what I have now. |
@nicolas-grekas Hi again, This issue is still continuing for me. I am working on this issue for a very long amount of time without success. I did install a duplicate test site(Symfony 6.0.3) on our normal test server which is not a container and the application works properly. Seeing that, I have tried to replace the php8.1-fpm-alpine with a different container php8.1-fpm. I cannot find any information regarding this, and not having the error itself even if it PHP crash I do not know what to do next. Is it possible that we will communicate and I will give you access to my machine and we check this together ? Any help will be very appreciated here. |
This is definitely a php bug, please report it there. |
Yes, but what do I report there ? |
You have a reproducer and a workaround, isn't it? That's a very good start. If you can try reducing the reproducer to something as minimal as possible, ppl on php-src would appreciate. |
You are correct, the issue now is when I access the Repository and do a retrieve action inside the cache->get(...) |
I'm afraid we won't be able to do anything here, but that's definitely something worth investigating and reporting to php-src. Please link back here when you'll open a ticket about the issue on php-src. |
@nicolas-grekas OK, this is deffently the issue. When debuging it, the files are crated to some point, and than it stops and throws that error.
I really did all I can here, it's up to you if you want to look into it or not. I do know the code has not changed for this file specificaly between version 5 to 6. |
@nicolas-grekas I don't see anything to report to them. It is thrown when trying to include a file that does not exists. |
The fact that the exception cannot be caught is what you might need to report. The code should be fine without the file_exists check, which is not needed. |
True, and it could be a separate issue. Would you consider patching that work around anyways so I can upgrade and continue to work ? |
I don't think committing a workaround in Symfony would be a good idea, unless we precisely understand the issue and decide we have no choice. But at this stage, reporting+fixing this on php-src is what would be the most useful for the php community. |
@nicolas-grekas I submitted bug to php rpc Here In symfony 6 it throws segment 11 fault that is thrown, the ltrace, strace didn't give me any clues or understanding whats behind it. Also strange no one else complained about it. |
Hi guys! composer install symfony/cache test.php <?php
use Symfony\Component\Cache\Adapter\FilesystemAdapter;
require __DIR__ . '/vendor/autoload.php';
$cache = new FilesystemAdapter();
$cache->get('wrong@key', function () {}); docker run -it --rm -v $PWD:/app -w /app php:8.1.2-cli bash
pecl install xdebug-3.1.3
docker-php-ext-enable xdebug
php test.php The error only reproduces with:
The error comes from creating \Closure here with a class with static variables here Bug is already fixed in PHP and Xdebug I think that we can fix it in the library code because it will fix the error and I think that using static variables in class functions is not a good practice. If you agree with me, I made a PR |
…of static variables (mrsuh) This PR was submitted for the 5.4 branch but it was merged into the 4.4 branch instead. Discussion ---------- [Cache] make LockRegistry use static properties instead of static variables | Q | A | ------------- | --- | Branch? | 4.4 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #44954 | License | MIT Commits ------- 3b6a56d [Cache] make LockRegistry use static properties instead of static variables
Symfony version(s) affected
5.4.2
Description
Hello,
After upgrading to version 5.4.2 the cache service stopped working. I could not trace it to why.
The configuration are working when I downgrade back to 5.4.0.
Unfrtunatly there is no exception that I can catch, and not enough information from nginx, php logs.
Just 502 error from nginx and could not xdebug it, had issues with it.
I went over the documentations and did not see any update that needs to be done to the code or deprecations.
Forgive me for not having enough information accept what I wrote.
Thank you
How to reproduce
Here is the configuration file:
`framework:
cache:
default_redis_provider: 'redis://%env(REDIS_HOST)%:%env(int:REDIS_PORT)%'
`
I inject it normally view constructor and pool name:
public function __construct( private LoggerInterface $logger, private TagAwareCacheInterface $redisCache ) { }
Usage sample:
$this->redisCache->get(self::CACHE_KEY, function (ItemInterface $item) { $item->expiresAfter(3600); $item->tag(['settings', Core::APP_CACHE_TAG]); ... }
Possible Solution
No response
Additional Context
No response
The text was updated successfully, but these errors were encountered: