Skip to content

Multiple driver errors caused by invalid return type of driverRead() #862

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

Closed
ShockedPlot7560 opened this issue Apr 12, 2022 · 32 comments · Fixed by #863
Closed

Multiple driver errors caused by invalid return type of driverRead() #862

ShockedPlot7560 opened this issue Apr 12, 2022 · 32 comments · Fixed by #863

Comments

@ShockedPlot7560
Copy link

ShockedPlot7560 commented Apr 12, 2022

Configuration

  • PhpFastCache version: 9.0.2
  • PhpFastCache API version: 4.0.0
  • PHP version: 8.0.13
  • Operating system: Ubuntu

Describe the bug

I don't really know the nature of the bug, but it seems to come from the File driver Phpfastcache\Drivers\Files\Driver::driverRead()

To Reproduce
Code to reproduce the issue:

/** @var ExtendedCacheItemPoolInterface */
$cacheManager = $this->container->get(CacheManager::class);
$cacheString = "SimplyString";
$cacheItem = $cacheManager->getItem($cacheString);

Driver used : files

Stacktrace

TypeError: Phpfastcache\Drivers\Files\Driver::driverRead(): Return value must be of type ?array, bool returned in /var/www/html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Files/Driver.php:74
Stack trace:
#0 /var/www/html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(132): Phpfastcache\Drivers\Files\Driver->driverRead(Object(Phpfastcache\Drivers\Files\Item))
#1 /var/www/html/src/core/middleware/MaintenanceMiddleware.php(32): Phpfastcache\Drivers\Files\Driver->getItem('MaintenanceAuto...')
...
#7 {main}
@github-actions
Copy link

Hello curious contributor !
Since it seems to be your first contribution, make sure that you've been:

  • Reading and searching out our WIKI
  • Reading and agreed with our Code Of Conduct
  • Reading and understood our Coding Guideline
  • Reading our README
    If everything looks unclear to you, tell us what 😄
    The Phpfastcache Team

@aemla
Copy link

aemla commented Apr 12, 2022

I have the same issue with Memcached driver.

  • PhpFastCache version: 9.1.0
  • PHP version: 8.0.17
  • Operating system: Ubuntu
PHP Fatal error:  Uncaught TypeError: Phpfastcache\Drivers\Memcached\Driver::driverRead(): Return value must be of type ?array, string returned in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php:154
Stack trace:
#0 /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(132): Phpfastcache\Drivers\Memcached\Driver->driverRead()
#1 /public_html/controller/Controller.php(61): Phpfastcache\Drivers\Memcached\Driver->getItem()
...
#4 {main}
  thrown in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php on line 154

@Geolim4
Copy link
Member

Geolim4 commented Apr 12, 2022

This behavior is very surprising as all tests would have been broken if such behavior would happens.

Did you decorated Phpfastcache services or something ?

If you see in /tests lot of call to getItem are made, especially with Files driver.

(See ReadWriteOperations.test.php)

@Geolim4
Copy link
Member

Geolim4 commented Apr 12, 2022

Can you please post your full code implementation below ?

Because this is the very first time report me this kind of issue that should be impossible to produce, especially with LOT of tests calling getItem().

@Geolim4
Copy link
Member

Geolim4 commented Apr 12, 2022

I have the same issue with Memcached driver.

* **PhpFastCache version:** 9.1.0

* **PHP version:** 8.0.17

* **Operating system:** Ubuntu
PHP Fatal error:  Uncaught TypeError: Phpfastcache\Drivers\Memcached\Driver::driverRead(): Return value must be of type ?array, string returned in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php:154
Stack trace:
#0 /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(132): Phpfastcache\Drivers\Memcached\Driver->driverRead()
#1 /public_html/controller/Controller.php(61): Phpfastcache\Drivers\Memcached\Driver->getItem()
...
#4 {main}
  thrown in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php on line 154

Can't reproduce it neither, I'm gonna need more information with your full code implementation.
For confidentiality you can send me sensible code at contact at geolim4.com
image

@ShockedPlot7560
Copy link
Author

Hello,
The error also seems very strange to me and never happens in local but only in production.
I will detail below the architecture likely to make a bug and will take the simplest case which brings me this error :

Before anything, I use the PHP-DI container for dependency injection that I build like this:

$buildeur = new ContainerBuilder();
$buildeur->addDefinitions($pathToConfigFile);
$buildeur->enableCompilation(__ROOT_FOLDER . "/tmp");
$container = $buildeur->build();

The configuration file is of type:

<?php

use DI\Container;
use Phpfastcache\CacheManager;
use Phpfastcache\Config\ConfigurationOption;

require __ROOT_FOLDER . "/vendor/autoload.php";

return [
	CacheManager::class => function (Container $container) {
		$instance = $container->get("ENV") === "dev" ? "Devnull" : "files";
		CacheManager::setDefaultConfig(new ConfigurationOption([
			"path" => $container->get("tmp.path")
		]));
		return CacheManager::getInstance($instance);
	}
];

After that, so the compilation of the container has been defined, and phpfastcache and DI share the same tmp/ directory.
A case that I have a problem with is going to be below. Or, the code that seems to be problematic anyway, a very trivial value retrieval and saving code.

//I have simplified the operation and removed the portions of code that are not necessary and do not interfere
$pdo = $container->get(PDO::class);
/** @var ExtendedCacheItemPoolInterface */
$cacheManager = $container->get(CacheManager::class);
$cacheString = "Maintenance";
$cacheItem = $cacheManager->getItem($cacheString);
if ($config["maintenance"] !== true && $pdo !== null) {
	$cacheItem->set(false)->expiresAfter(3600 * 24);
	$cacheManager->save($cacheItem);
} else {
	if($pdo === null){
		if($cacheItem->get() === false || $cacheItem->get() === null){
			// send notification to discord
		}
		if(!$cacheItem->isHit()){
			$cacheItem->set(true)->expiresAfter(3600 * 24);
		}
		$cacheManager->save($cacheItem);
	}
}

@Geolim4
Copy link
Member

Geolim4 commented Apr 12, 2022

I think you are facing race condition issue with Files driver.
Try to enable the option preventCacheSlams (only for Files driver), see Wiki.

Also I suggest you a better approach with the CacheManager.

Instead of calling:

		CacheManager::setDefaultConfig(new ConfigurationOption([
			"path" => $container->get("tmp.path")
		]));

Try to do:

		return CacheManager::getInstance($instance, [
		    'path' => $container->get("tmp.path"),
		    'preventCacheSlams' => true,
                ]);
                // OR

Also try a more performant driver like Redis if you can, this will be more appropriated for heavy cache usage.

@ShockedPlot7560
Copy link
Author

I'll try your answer, however regarding the Redis driver, it takes, I guess a lot of memory space?

@Geolim4
Copy link
Member

Geolim4 commented Apr 13, 2022 via email

@ShockedPlot7560
Copy link
Author

Update after testing in production (where it crashes). The error is always launched having set the configuration as requested. I will try to switch to the Redis driver to see if it still happens or not.

@Geolim4
Copy link
Member

Geolim4 commented Apr 13, 2022

How many request per second do you have ?

Can you invit me to talk on discord maybe ? :)

@ShockedPlot7560
Copy link
Author

This can go up I think to 10/15 requests per second at most. They are then limited by the capacity of my proxy and the traffic.

Of course, here is my discord: ShockedPlot7560 # 9999

@Geolim4
Copy link
Member

Geolim4 commented Apr 13, 2022

Added you, so we can discuss more clearly about your issue !

@Geolim4
Copy link
Member

Geolim4 commented Apr 13, 2022

I have the same issue with Memcached driver.

* **PhpFastCache version:** 9.1.0

* **PHP version:** 8.0.17

* **Operating system:** Ubuntu
PHP Fatal error:  Uncaught TypeError: Phpfastcache\Drivers\Memcached\Driver::driverRead(): Return value must be of type ?array, string returned in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php:154
Stack trace:
#0 /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(132): Phpfastcache\Drivers\Memcached\Driver->driverRead()
#1 /public_html/controller/Controller.php(61): Phpfastcache\Drivers\Memcached\Driver->getItem()
...
#4 {main}
  thrown in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php on line 154

I need more information @aemla. What is the string returned exactly ?
Can you dump me the content of that string please ? Because unlike @ShockedPlot7560 I can't reproduce your issue.

@aemla
Copy link

aemla commented Apr 14, 2022

I have the same issue with Memcached driver.

* **PhpFastCache version:** 9.1.0

* **PHP version:** 8.0.17

* **Operating system:** Ubuntu
PHP Fatal error:  Uncaught TypeError: Phpfastcache\Drivers\Memcached\Driver::driverRead(): Return value must be of type ?array, string returned in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php:154
Stack trace:
#0 /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(132): Phpfastcache\Drivers\Memcached\Driver->driverRead()
#1 /public_html/controller/Controller.php(61): Phpfastcache\Drivers\Memcached\Driver->getItem()
...
#4 {main}
  thrown in /public_html/vendor/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Memcached/Driver.php on line 154

I need more information @aemla. What is the string returned exactly ? Can you dump me the content of that string please ? Because unlike @ShockedPlot7560 I can't reproduce your issue.

It can be reproduced by running it in a loop. I wrote a little example, it doesn't make any sense, but it throws the same error. 😁 In my original code, I am getting data from a database.
I am getting this error only with the Memcached driver. Files driver works fine.

Dump of that string:
string(0) ""

    function test()
    {
        $array = [
            ['userId' => 1, 'data' => 'test'],
            ['userId' => 2, 'data' => 'test'],
            ['userId' => 3, 'data' => 'test'],
            ['userId' => 4, 'data' => 'test'],
            ['userId' => 5, 'data' => 'test'],
            ['userId' => 6, 'data' => 'test'],
            ['userId' => 7, 'data' => 'test'],
            ['userId' => 8, 'data' => 'test'],
            ['userId' => 9, 'data' => 'test'],
            ['userId' => 10, 'data' => 'test'],
            ['userId' => 11, 'data' => 'test'],
            ['userId' => 12, 'data' => 'test'],
            ['userId' => 13, 'data' => 'test'],
            ['userId' => 14, 'data' => 'test'],
            ['userId' => 15, 'data' => 'test'],
            ['userId' => 16, 'data' => 'test'],
            ['userId' => 17, 'data' => 'test'],
            ['userId' => 18, 'data' => 'test'],
            ['userId' => 19, 'data' => 'test'],
            ['userId' => 20, 'data' => 'test'],
            ['userId' => 21, 'data' => 'test'],
            ['userId' => 22, 'data' => 'test'],
        ];

        $cache = CacheManager::getInstance('Memcached');

        // The error only occurs when writing to the cache, so let's clear it before running the loop.
        $cache->clear();

        foreach ($array as $row) {
            $this->getFromCache($cache, $row['userId'], $row['data']);
        }
    }

    function getFromCache(ExtendedCacheItemPoolInterface $cache, $userId, $data)
    {
        $cachedString = $cache->getItem('dataById=' . $userId);
        if (!$cachedString->isHit()) {
            $cachedString->set($data)->expiresAfter(300);
            $cache->save($cachedString);

            return $cachedString->get();
        } else {
            return $cachedString->get();
        }
    }

@Geolim4
Copy link
Member

Geolim4 commented Apr 14, 2022

I ran your code sample:

image

And I have no error at all, so I can't reproduce your bug :/

@Geolim4
Copy link
Member

Geolim4 commented Apr 14, 2022

What version of php-memcached version are you using ?
Do you run multiple concurrent process ?

@aemla
Copy link

aemla commented Apr 14, 2022

This error occurs only in 9.x, it works fine in 8.x. I am using LSMCD https://github.com/litespeedtech/lsmcd could this be the reason why it sometimes returns an empty string?

@aemla
Copy link

aemla commented Apr 14, 2022

This issue can be fixed by adding a check for an empty string.

    /**
     * @param ExtendedCacheItemInterface $item
     * @return null|array
     */
    protected function driverRead(ExtendedCacheItemInterface $item): ?array
    {
        $val = $this->instance->get($item->getKey());

        if ($val === false || $val === “”) {
            return null;
        }

        return $val;
    }

@Geolim4
Copy link
Member

Geolim4 commented Apr 14, 2022

"it works fine in 8.x" ?

That's interesting. Did you tried in v8.1.x or previous one ?

@Geolim4
Copy link
Member

Geolim4 commented Apr 14, 2022

I understand now, I typed driverRead with ?array as of v9.

@Geolim4
Copy link
Member

Geolim4 commented Apr 14, 2022

Ok ok, that make sense now.

v8 was workin well because I previously made a weak validation:

But as of v9 I introduced (very) strict types everywhere this one can be problematic under some circumstances.

@Geolim4
Copy link
Member

Geolim4 commented Apr 14, 2022

Just out of curiosity tell me if replacing:

        if ($val === false) {
            return null;
        }

by

        if (empty($val) || !\is_array($val)) {
            return null;
        }

fixes your issue ?

@aemla
Copy link

aemla commented Apr 14, 2022

Just out of curiosity tell me if replacing:

        if ($val === false) {
            return null;
        }

by

        if (empty($val) || !\is_array($val)) {
            return null;
        }

fixes your issue ?

Yes, now it works fine.

@Geolim4
Copy link
Member

Geolim4 commented Apr 14, 2022

Roger that. I'll push a fix tonight and make a new release by the end of the week at late.

@Geolim4 Geolim4 changed the title Unexpected return of driverRead() Multiple driver errors caused by invalid return type of driverRead() Apr 15, 2022
@Geolim4 Geolim4 mentioned this issue Apr 15, 2022
6 tasks
@GrandFelix
Copy link

Hi,

we are still getting the same error. We are using version 9.1.2 and php 8.1.

PHP Fatal error:  Uncaught TypeError: Phpfastcache\Drivers\Files\Driver::driverRead(): Return value must be of type ?array, bool returned in /var/www/prod/app/vendor8/phpfastcache/phpfastcache/lib/Phpfastcache/Drivers/Files/Driver.php:74
Stack trace:
#0 /var/www/prod/app/vendor8/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(143): Phpfastcache\Drivers\Files\Driver->driverRead()
#1 /var/www/prod/app/vendor8/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(106): Phpfastcache\Drivers\Files\Driver->getItem()
#2 /var/www/prod/app/vendor8/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/TaggableCacheItemPoolTrait.php(399): Phpfastcache\Drivers\Files\Driver->getItems()
#3 /var/www/prod/app/vendor8/phpfastcache/phpfastcache/lib/Phpfastcache/Core/Pool/CacheItemPoolTrait.php(386): Phpfastcache\Drivers\Files\Driver->driverWriteTags()
#4 /var/www/prod/app/library/App/Cache/Cache.php(76): Phpfastcache\Drivers\Files\Driver->save()

Any tip?

@Geolim4
Copy link
Member

Geolim4 commented May 3, 2023

You seem to have concurrency issue as the file exists on disk but looks empty, very weird.

Did you tried to enable preventCacheSlams option ?

@GrandFelix
Copy link

will try and I report back.

@GrandFelix
Copy link

We have tried it and we still get the same error. I think the problem is in decode() function when calling unserialize() which one return false when it cant unserialize a string.

@Geolim4
Copy link
Member

Geolim4 commented May 17, 2023

Then you have a serious problem of I/O with your disk this is is not supposed to happens unless the file are corrupted or partially written which is a sign of a failing disk. I have multiple tests with this driver on the CI including concurrent writes and I'm not encountering this kind of issue.

@GrandFelix
Copy link

The problem is that this happens on multiple different servers and I really doubt that all servers have disk issues 😕

@GrandFelix
Copy link

now we have tired to set setSecureFileManipulation() and it looks good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants