Problem/Motivation

If you have a widget form with #ajax, something that page manager itself might also have, then you get a container was serialized error and then a fatal error.

Proposed resolution

Add some dark magic in __sleep() to unset properties that might contain the container and/or can be recreatd.

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

FileSize
595 bytes

Status: Needs review » Needs work

The last submitted patch, 1: page-manager-2419081-1.patch, failed testing.

Status: Needs work » Needs review
Berdir’s picture

Issue tags: +Needs tests
FileSize
2.39 KB

Ouch, this definitely needs tests.

This seems to be working for me for both adding and editing widgets. Was a tough one to debug and figure out, very weird php serialization issues, similar to what I recently saw when fixing search_api. $this->block in validate for the add form was suddenly just a string.

And for edit, the problem was that the page entity had a different display collection than we had in the form, as that one was still the serialized version, and then it didn't get updated.

I think it would be better to not store the block/display object at all as properties. The effort to create them again can't be much bigger than the overhead of serializing those possibly huge objects.

neclimdul’s picture

  1. +++ b/src/Entity/Page.php
    @@ -300,4 +300,25 @@ class Page extends ConfigEntityBase implements PageInterface {
    +    // Avoid serializing plugin collections, collection as they might contain
    

    "collections, collection"?

  2. +++ b/src/Entity/Page.php
    @@ -300,4 +300,25 @@ class Page extends ConfigEntityBase implements PageInterface {
    +    $unset_vars = array('variantCollection' => 'display_variants', 'accessConditionCollection' => 'access_variants', 'executable' => NULL);
    ...
    +        unset($vars[array_search($unset_var, $vars)]);
    

    I guess this is a whitelist of sorts. Excuse me if my lack of Entity experience is showing but these make me nervous. Are we sure there aren't other things that might sneak in from contrib? Should it be smarter somehow?

Berdir’s picture

Thanks for the review.

2. I don't think this is a problem. Entity classes are just like other classes, contrib can't just add more properties there. Config entities can only be extended in very strict ways ways, and those things are just scalars/arrays, nothing that we need to worry about when serializing.

Berdir’s picture

The last submitted patch, 7: page-manager-2419081-7-test-only.patch, failed testing.

Berdir’s picture

Issue tags: -Needs tests
tim.plunkett’s picture

Status: Needs review » Fixed

Thanks @Berdir!

  • tim.plunkett committed 389e915 on 8.x-1.x authored by Berdir
    Issue #2419081 by Berdir: Container is being serialized if you have a...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.