Fixed Argument 1 passed to XF\Template\Templater::setGroupStyles() must be of the type array

[Nirjonadda]

Sorry, for misunderstanding. Conversation are my fault to typing. It's should be not getting error.

 

[420]

Bugs aren’t necessarily handled as soon as they come in so if it’s something that needs a reply then posting elsewhere on the forum or submitting a ticket is preferable.

That said, Redis is fully supported but indeed at the time this request happened, some data either wasn’t available in Redis or came out in an unexpected format. This can happen for example if Redis was restarted unexpectedly. If the errors were a one off, then they can be safely ignored. If they are persistent, I’d recommend looking at whether Redis itself is functioning properly or if there are reasons that might explain that data not being available at that time, such as it becoming unavailable or unable to connect for some reason.

@M@rc have you discussed this with Steven, as it appears XF does not acknowledge the error, yet I believe it's still happening.

He told me Redis was setup correctly and everything was fine.

Thank you.

 

[Steffen]

I'm getting similar errors that indicate that sometimes false is returned from the cache (instead of null).

XenForo uses the Doctrine RedisCache to store serialized PHP values in Redis. I think there is a race condition in this library: https://github.com/doctrine/cache/blob/master/lib/Doctrine/Common/Cache/RedisCache.php#L60

RedisCache calls "mget" to fetch multiple values. If one of these values is returned as false then it double-checks whether the value really doesn't exist by calling the "exists" method on the key. Now, if the value has just been (re-)created between the "mget" and the "exists" call then it thinks that false is the value of the specified key (although it isn't).

This is a conceptional issue of letting PhpRedis serialize the data (instead of doing that in the RedisCache library). With serialization enabled, PhpRedis returns false for a) keys that don't exist and b) for keys that exist and have the value false. To distinguish between these two cases, RedisCache uses the "exists" call which unfortunately introduces the described race condition.

Is false a legitimate cache value in XenForo? If not then the "exists" call could just be removed. The other alternative would be to modify RedisCache such that it does the serialization on its own.

TypeError: Argument 1 passed to XF\SimpleCache::__construct() must be of the type array, boolean given, called in src/XF/App.php on line 549 src/XF/SimpleCache.php:12

Stack trace
#0 src/XF/App.php(549): XF\SimpleCache->__construct(false)
#1 src/XF/Container.php(28): XF\App->XF\{closure}(Object(XF\Container))
#2 src/XF/App.php(2150): XF\Container->offsetGet('simpleCache')
#3 src/XF/App.php(1678): XF\App->simpleCache()
#4 src/XF/App.php(1601): XF\App->getGlobalTemplateData(Object(XF\Mvc\Reply\View))
#5 src/XF/Pub/App.php(350): XF\App->preRender(Object(XF\Mvc\Reply\View), 'json')
#6 src/XF/Mvc/Dispatcher.php(280): XF\Pub\App->preRender(Object(XF\Mvc\Reply\View), 'json')
#7 src/XF/Mvc/Dispatcher.php(44): XF\Mvc\Dispatcher->render(Object(XF\Mvc\Reply\View), 'json')
#8 src/XF/App.php(1945): XF\Mvc\Dispatcher->run()
#9 src/XF.php(328): XF\App->run()
#10 index.php(13): XF::runApp('XF\\Pub\\App')
#11 {main}

Uncaught TypeError: Argument 1 passed to XF\App::XF\{closure}() must be of the type array, boolean given, called in src/XF/App.php on line 1460 and defined in src/XF/App.php:555

Stack trace:
#0 src/XF/App.php(1460): XF\App->XF\{closure}(false, Object(XF\Container), 'options')
#1 src/XF/Container.php(28): XF\App->XF\{closure}(Object(XF\Container))
#2 src/XF/App.php(2158): XF\Container->offsetGet('options')
#3 src/XF.php(463): XF\App->options()
#4 src/XF/Entity/User.php(1614): XF::options()
#5 src/XF/Mvc/Entity/Manager.php(70): XF\Entity\User::getStructure(Object(XF\Mvc\Entity\Structure))
#6 src/XF/Repository/User.php(60): XF\Mvc\Entity\Manager->getEntityStructure('XF:User')
#7 src/XF/Repository/User.php(31): XF\Repository\User->getGuestUser()
#8 src/XF.php(367): XF\Repository\User->getVisitor(0)
#9 src/XF/Error.php(132): XF::visitor()
#10 src/XF/App.php(1956): XF\Error->logException(Object(ErrorException), false, '')
#11 src/XF.php(184): XF\App->logException(Object(ErrorException))
#12 [internal function]: XF::handleFatalError()
#13 {main}
  thrown in src/XF/App.php on line 555

 

[Steffen]

I have reported the issue on GitHub: https://github.com/doctrine/cache/issues/258

 

[Chris D]

Thanks Steffen, very interesting.

We'll keep an eye on this.

 

[Xon]

To be honest, this is something of a phpredis issue. Redis actually returns what is better considered "null" rather than "false". This isn't the first time phpredis has miss-handled return types. But changing it's behaviour is a massive backwards compatibility breaking change.

 

[Steffen]

Yes, it is an issue with the built-in serialization feature of PhpRedis.

When using the built-in serialization, the underlying issue is that PhpRedis makes it impossible to distinguish between a non-existing value and the value false. When not using the built-in serialization, this issue does not exist because in this case get/mget either returns a string or false.

I therefore think that RedisCache should avoid using the built-in serialization feature of PhpRedis and do the serialization on its own. I agree that changing the behaviour of PhpRedis would be a massive headache. I'd therefore just not use the problematic parts of PhpRedis (the serialization feature).

 

[Xon]

@Steffen that is actually what I do in my Redis Cache add-on (for XF2) Which is a thin wrapper on phpredis with a pure-php compat alternative

 

[XF Bug Bot]

Thank you for reporting this issue. The issue is now resolved and we are aiming to include that in the next XF release (2.0.10).

Change log:

Fix a potential race condition and return type ambiguity in the Doctrine RedisCache provider. Based on code contributed to Doctrine by @Steffen (thank you!)

 

[420]

Thanks Chris!

 

[Xon]

I have to say blocking a fix for a race condition because a maintenance developer wants a unit test to cover a race condition and that their framework is single threaded is particularly silly.

 

[Chris D]

Indeed. Quite disappointed in their stance to say the least.