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.

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

[4.x]: unserialize() Error at offset #13164

Closed
croxton opened this issue May 6, 2023 · 22 comments
Closed

[4.x]: unserialize() Error at offset #13164

croxton opened this issue May 6, 2023 · 22 comments

Comments

@croxton
Copy link

croxton commented May 6, 2023

What happened?

Description

The storage/runtime cache becomes corrupted when saving an entry using the keyboard (control-s) when Craft is in the middle of auto-saving the entry. The entire site is broken until the cache is deleted.

This problem seems to have emerged with the latest (or a recent) version of Craft, but I may have just missed it previously.

Steps to reproduce

  1. Make an edit to an entry with a lot of field data, and quickly hit control-s while the auto-save is spinning; this will trigger the error.

Expected behavior

Manual saving should be disabled during autosave, or the manual save should be queued.

Actual behavior

Entire site breaks!

Craft CMS version

4.4.9

PHP version

8.1.10 (MAMP Pro on OSX)

Operating system and version

OSX

Database type and version

mysql 5.7.34 (MAMP Pro on OSX)

Image driver and version

No response

Installed plugins and versions

Calendar Links
Control Panel JS
Element API
Environment Label
Feed Me
Field Manager
Freeform
Google Maps
Help Links
Hyper
Image Resizer
Imgixer
Matrix Mate
Navigation
Postmark
Read Time
Redactor
SEOmatic
Servd Assets & Helpers
Sherlock
Similar
Sprig
Super Table
Table Maker
Template Select

@croxton
Copy link
Author

croxton commented May 6, 2023

When the cache is corrupted, there is a unusually large .bin file in storage/runtime/cache/8e (always the same subdirectory 8e) with a huge block of <0x00>s in it (but not in the same place):

Screenshot 2023-05-06 at 12 55 56

Screenshot 2023-05-06 at 12 56 06

Screenshot 2023-05-04 at 18 10 22-b

@croxton
Copy link
Author

croxton commented May 6, 2023

That Yii caching code looks like it should be more error tolerant when unserializing whatever the cause of this is, no?

@brandonkelly
Copy link
Member

brandonkelly commented May 6, 2023

The only cache-related thing Craft does when saving an entry is invalidate existing caches for the element, which doesn’t actually touch the element caches at all—only cache tag caches are updated, which are very small cache files that only contain a timestamp. So whatever caused this corruption, it wasn’t caused by saving an entry twice in quick succession (at least directly).

That cache looks like it contains a serialized field layout. Probably from an entire entry getting cached? It’s generally not a good idea to cache instantiated PHP objects unless the object’s class was clearly coded for it (e.g. with __sleep() and __wakeup() methods), which FieldLayout and Entry were not. The cache data would be significantly larger than typical caches, and more error-prone if changes occur in any of the serialized classes between the time the cache is created and retrieved.

So you should probably try to track down why the field layout/entry is getting cached in the first place, and cache the specific data you need from it instead.

That Yii caching code looks like it should be more error tolerant when unserializing whatever the cause of this is, no?

Yeah, wrapping that code in a try/catch wouldn’t hurt. Just PR’d that change: yiisoft/yii2#19827

@croxton
Copy link
Author

croxton commented May 6, 2023

So you should probably try to track down why the field layout/entry is getting cached in the first place, and cache the specific data you need from it instead.

This isn't something I'm doing anywhere (in my own code at least), so maybe it's one of those plugins. I'll see if I can track down the culprit.

Yeah, wrapping that code in a try/catch wouldn’t hurt. Just PR’d that change: yiisoft/yii2#19827

Great, thank you!

@croxton
Copy link
Author

croxton commented May 6, 2023

Sorry Brandon, I disabled plugins in the config ('disabledPlugins' => '*') and found that a 1.1Mb file was still generated in the storage/runtime/cache/8e folder, which I assume must be generated by Craft's own code? Checking other projects running the latest Craft I also see a similar file in that same folder (looks like a field layout), but not on projects running slightly older version of Craft. I'm going to roll back to a previous version of this project and an older version of Craft to see if that file is still generated.

@croxton
Copy link
Author

croxton commented May 6, 2023

Went back to Craft 4.3.6.1 and that 1.1Mb cache file is still generated in that version of Craft. As far as I can see this is a cache file generated by Craft itself for all of the field layouts. It has an expiry time of 1 day. If I delete it, Craft will recreate it on any request in the CP or frontend, or just by running ./craft on the command line. Presumably an entry save could trigger a recreation of this cache if it was the first request after it has expired. Could that cause a race condition?

@brandonkelly
Copy link
Member

Hm, I’m not seeing the same behavior on my end.

Try the following:

  1. Change your craftcms/cms requirement in composer.json to dev-develop#385516ee99b8b4b5105d62ae32bbadb9bda6a7d5 (so you can get the new ArrayHelper::containsRecursive() method I just added in 385516e)
  2. Open vendor/yiisoft/yii2/caching/Cache.php and add the following code to the beginning of the set() method:
    if (
        $value instanceof \craft\base\ElementInterface ||
        $value instanceof \craft\base\FieldInterface ||
        (
            is_array($value) &&
            \craft\helpers\ArrayHelper::containsRecursive($value, fn($e) => 
                $e instanceof \craft\base\ElementInterface ||
                $e instanceof \craft\base\FieldInterface
            )
        )
    ) {
        \Craft::$app->getErrorHandler()->logException(new \Exception('CACHE ISSUE'));
    }
  3. Clear caches and save an entry or whatever causes the value to get cached.
  4. Search your logs in storage/logs/ for CACHE ISSUE.

There should be a stack trace along with the log message. Can you post it here?

If the exception isn’t getting logged, maybe some other object is getting cached that contains the field layout. You could change the code to this:

```php
if (
    (
        $value instanceof \yii\base\BaseObject &&
        !$value instanceof \yii\web\UrlRule
    ) ||
    (
        is_array($value) &&
        \craft\helpers\ArrayHelper::containsRecursive($value, fn($e) =>
            $e instanceof \yii\base\BaseObject &&
            !$e instanceof \yii\web\UrlRule
        )
    )
) {
    \Craft::$app->getErrorHandler()->logException(new \Exception('CACHE ISSUE'));
}
```

There could be additional noise with that code, though.

@croxton
Copy link
Author

croxton commented May 6, 2023

The second method identified nystudio107/craft-autocomplete:

2023-05-06 20:16:57 [console.ERROR] [Exception] Exception: CACHE ISSUE in [redacted]/vendor/yiisoft/yii2/caching/Cache.php:254
Stack trace:
#0 [redacted]/vendor/yiisoft/yii2/caching/Cache.php(624): yii\caching\Cache->set('seomatic_metaco...', Array, 30, Object(yii\caching\TagDependency))
#1 [redacted]/vendor/nystudio107/craft-seomatic/src/services/MetaContainers.php(444): yii\caching\Cache->getOrSet('seomatic_metaco...', Object(Closure), 30, Object(yii\caching\TagDependency))
#2 [redacted]/vendor/nystudio107/craft-seomatic/src/twigextensions/SeomaticTwigExtension.php(54): nystudio107\seomatic\services\MetaContainers->loadMetaContainers('/', 1)
#3 [redacted]/vendor/twig/twig/src/ExtensionSet.php(320): nystudio107\seomatic\twigextensions\SeomaticTwigExtension->getGlobals()
#4 [redacted]/vendor/twig/twig/src/Environment.php(789): Twig\ExtensionSet->getGlobals()
#5 [redacted]/vendor/nystudio107/craft-autocomplete/src/generators/AutocompleteVariableGenerator.php(80): Twig\Environment->getGlobals()
#6 [redacted]/vendor/nystudio107/craft-autocomplete/src/generators/AutocompleteVariableGenerator.php(57): nystudio107\autocomplete\generators\AutocompleteVariableGenerator::generateInternal()
#7 [redacted]/vendor/nystudio107/craft-autocomplete/src/Autocomplete.php(143): nystudio107\autocomplete\generators\AutocompleteVariableGenerator::generate()
#8 [internal function]: nystudio107\autocomplete\Autocomplete->generateAutocompleteClasses(Object(yii\base\Event))
#9 [redacted]/vendor/yiisoft/yii2/base/Event.php(312): call_user_func(Array, Object(yii\base\Event))
#10 [redacted]/vendor/yiisoft/yii2/base/Component.php(642): yii\base\Event::trigger('craft\\services\\...', 'afterLoadPlugin...', Object(yii\base\Event))
#11 [redacted]/vendor/craftcms/cms/src/services/Plugins.php(275): yii\base\Component->trigger('afterLoadPlugin...')
#12 [redacted]/vendor/craftcms/cms/src/base/ApplicationTrait.php(1554): craft\services\Plugins->loadPlugins()
#13 [redacted]/vendor/craftcms/cms/src/console/Application.php(52): craft\console\Application->_postInit()
#14 [redacted]/vendor/yiisoft/yii2/base/BaseObject.php(109): craft\console\Application->init()
#15 [redacted]/vendor/yiisoft/yii2/base/Application.php(204): yii\base\BaseObject->__construct(Array)
#16 [redacted]/vendor/yiisoft/yii2/console/Application.php(89): yii\base\Application->__construct(Array)
#17 [internal function]: yii\console\Application->__construct(Array)
#18 [redacted]/vendor/yiisoft/yii2/di/Container.php(419): ReflectionClass->newInstanceArgs(Array)
#19 [redacted]/vendor/yiisoft/yii2/di/Container.php(170): yii\di\Container->build('craft\\console\\A...', Array, Array)
#20 [redacted]/vendor/yiisoft/yii2/BaseYii.php(365): yii\di\Container->get('craft\\console\\A...', Array, Array)
#21 [redacted]/vendor/craftcms/cms/src/Craft.php(54): yii\BaseYii::createObject(Array, Array)
#22 [redacted]/vendor/craftcms/cms/bootstrap/bootstrap.php(252): Craft::createObject(Array)
#23 [redacted]/vendor/craftcms/cms/bootstrap/console.php(40): require('/Users/markcrox...')
#24 [redacted]/craft(12): require('/Users/markcrox...')
#25 {main} {"memory":57719088,"exception":"[object] (Exception(code: 0): CACHE ISSUE at [redacted]/vendor/yiisoft/yii2/caching/Cache.php:254)"} 
2023-05-06 20:16:57 [console.INFO] [nystudio107\autocomplete\Autocomplete::generateAutocompleteClasses] Autocomplete classes generated {"memory":59210424} 

However, I uninstalled that from require-dev and ran it again. The large 1.1Mb is STILL generated. No CACHE ISSUE in logs for either of your methods so I just edited to throw an exception with no conditions just to see what was calling it, as there is only this one file being generated now. The stack trace indicates that craft\services\ProjectConfig->_loadInternalConfig() causes the the large cache file to be created in 8e:

2023-05-06 20:37:33 [console.ERROR] [Exception] Exception: CACHE ISSUE in [redacted]/vendor/yiisoft/yii2/caching/Cache.php:243
Stack trace:
#0 [redacted]/vendor/yiisoft/yii2/caching/Cache.php(613): yii\caching\Cache->set('projectConfig:i...', Array, NULL, Object(yii\caching\ExpressionDependency))
#1 [redacted]/vendor/craftcms/cms/src/services/ProjectConfig.php(1785): yii\caching\Cache->getOrSet('projectConfig:i...', Object(Closure), NULL, Object(yii\caching\ExpressionDependency))
#2 [redacted]/vendor/craftcms/cms/src/services/ProjectConfig.php(1707): craft\services\ProjectConfig->_loadInternalConfig()
#3 [redacted]/vendor/craftcms/cms/src/services/ProjectConfig.php(1721): craft\services\ProjectConfig->getInternalConfig()
#4 [redacted]/vendor/craftcms/cms/src/services/ProjectConfig.php(441): craft\services\ProjectConfig->getCurrentWorkingConfig()
#5 [redacted]/vendor/craftcms/cms/src/base/ApplicationTrait.php(1575): craft\services\ProjectConfig->get('system.timeZone')
#6 [redacted]/vendor/craftcms/cms/src/base/ApplicationTrait.php(1510): craft\console\Application->_setTimeZone()
#7 [redacted]/vendor/craftcms/cms/src/console/Application.php(50): craft\console\Application->_preInit()
#8 [redacted]/vendor/yiisoft/yii2/base/BaseObject.php(109): craft\console\Application->init()
#9 [redacted]/vendor/yiisoft/yii2/base/Application.php(204): yii\base\BaseObject->__construct(Array)
#10 [redacted]/vendor/yiisoft/yii2/console/Application.php(89): yii\base\Application->__construct(Array)
#11 [internal function]: yii\console\Application->__construct(Array)
#12 [redacted]/vendor/yiisoft/yii2/di/Container.php(419): ReflectionClass->newInstanceArgs(Array)
#13 [redacted]/vendor/yiisoft/yii2/di/Container.php(170): yii\di\Container->build('craft\\console\\A...', Array, Array)
#14 [redacted]/vendor/yiisoft/yii2/BaseYii.php(365): yii\di\Container->get('craft\\console\\A...', Array, Array)
#15 [redacted]/vendor/craftcms/cms/src/Craft.php(54): yii\BaseYii::createObject(Array, Array)
#16 [redacted]/vendor/craftcms/cms/bootstrap/bootstrap.php(252): Craft::createObject(Array)
#17 [redacted]/vendor/craftcms/cms/bootstrap/console.php(40): require('/Users/markcrox...')
#18 [redacted]/craft(12): require('/Users/markcrox...')
#19 {main} {"memory":22635624,"exception":"[object] (Exception(code: 0): CACHE ISSUE at [redacted]/vendor/yiisoft/yii2/caching/Cache.php:243)"} 
2023-05-06 20:37:33 [console.INFO] [modules\custommodule\CustomModule::init] custom module loaded {"memory":24671320} 

I had project config disabled, but I tried it with it enabled (after making sure the project config yaml was rebuilt from the database) - and the large cache file is still created with the stack trace the same.

@brandonkelly brandonkelly reopened this May 7, 2023
@brandonkelly
Copy link
Member

Any chance you can send a database backup and your Composer files over to support@craftcms.com?

@croxton
Copy link
Author

croxton commented May 7, 2023

I've sent that over, let me know if you need anything else. Many thanks for taking a look.

@brandonkelly
Copy link
Member

Looks like I was wrong, there aren’t any entries/field layouts getting cached. Just the field layout settings for an entry type, which is expected.

Still, saving an entry won’t touch your project config, so I don’t see how double-saving an entry would cause the project config cache to become corrupted. Probably just a coincidence.

The Yii PR was rejected since unserialize() will only trigger a PHP notice when there’s a problem, which isn’t supposed to halt execution.

So the real issue here is that Craft is being a little overzealous when it comes to error reporting. I’ve just toned that down for the next release. Going forward, PHP warnings and notices will no longer halt execution unless Dev Mode is disabled.

@croxton
Copy link
Author

croxton commented May 9, 2023

Thank you, that looks like the best solution possible for now.

For the record, this happened on three occasions when editing the same entry; each time the corrupted cache file had a modified date exactly 1 day after the time of the error - suggesting the file was indeed touched during the save process for the entry, inexplicable though that seems!

@brandonkelly
Copy link
Member

brandonkelly commented May 9, 2023

4.4.10 is out with the warning and notice reporting changes.

@croxton
Copy link
Author

croxton commented Jul 20, 2023

Would you mind reopening this issue, if appropriate? We're seeing problems with another project where the project config cache file (always saved at storage/runtime/cache/8e) seems unreasonably large - 1.4Mb. When making any change in the CP that writes PC data (editing a field, adding a user group etc) we get a 500 error as Craft tries to rebuild the cache - we think because that file gets corrupted. Increasing PHP memory etc doesn't help. This seems like an architecture problem - should all that data really be serialised in one go, would it be better broken up into smaller caches?

@croxton
Copy link
Author

croxton commented Jul 20, 2023

The cache duration also seems short - one day. Why not a year? Regeneration of the cache is triggered by any request, so given the potential for that file to be corrupted we could see intermittent errors on the front end for an unlucky user.

brandonkelly added a commit that referenced this issue Aug 2, 2023
@brandonkelly
Copy link
Member

@croxton The cache stores the project config data verbatim, so it would only grow that large if your project config itself is that large. The best way to reduce that would be to simplify your content model. (We’re working to make that more doable in Craft 5.)

should all that data really be serialised in one go, would it be better broken up into smaller caches?

Not a bad idea. Would you mind loading your config/project/ folder into a tool like DaisyDisk and showing me a breakdown of its nested folder/file sizes?

The cache duration also seems short - one day. Why not a year?

It just uses the default cache duration, which probably makes sense to be 24h by default, considering the variety of things that get cached. But I just added a way to override it from config/app.php for Craft 4.5 (57df57b):

use craft\helpers\App;
use craft\helpers\ConfigHelper;

return [
    'components' => [
        'projectConfig' => fn() => Craft::createObject([
            'cacheDuration' => ConfigHelper::durationInSeconds('P1Y'),
        ] + App::projectConfigConfig()),
    ],
];

@croxton
Copy link
Author

croxton commented Aug 10, 2023

@brandonkelly thank you for allowing the cache duration to be configurable - that should help a lot.

Here's a breakdown:
Screenshot 2023-08-10 at 08 56 30

Interestingly, looking at the largest files the yaml for Verbb's Hyper fields seems excessively verbose compared to other field types.

@croxton
Copy link
Author

croxton commented Aug 10, 2023

@brandonkelly If you are able to break the PC cache up into smaller files I think that could actually prove quite a crucial optimisation as the complexity of Craft fields (such as Hyper) increases. In terms of content model the sites in question are actually not as complex as other sites we have built (that remain on Craft 3).

@brandonkelly
Copy link
Member

I take it the Hyper field(s) live within matrixBlockTypes and superTableBlockTypes?

@croxton
Copy link
Author

croxton commented Aug 11, 2023

Yes that's right.

@croxton
Copy link
Author

croxton commented Aug 14, 2023

Just to follow up: by default Hyper adds an 'Advanced' tab containing 3 fields to every link type within a field, and whether that link type is enabled or disabled the field layout is saved in project config. That adds an additional 30 fields for each Hyper field created in the CMS. For the project in question it was fine for us to remove this tab for most Hyper fields. This is the result:

Screenshot 2023-08-14 at 10 45 37

A significant improvement I think. Thus far, I've not see any further 500 errors on cache generation following this change, and CP performance when making admin changes that write PC data is noticeably faster.

@brandonkelly
Copy link
Member

Craft 4.5.0 is out with the ProjectConfig::$cacheDuration property.

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

No branches or pull requests

2 participants