Problem/Motivation
The method tries to do two things:
a) It ensures that values for initialized plugin objects in plugin collections are stored
b) Preventing those plugin collections from being serialized by removing them from $vars.
The problem is that b) doesn't work if the plugin collection object was not previously initialized already.
I've seen this happening with filter config entities, which caches the filter config entities (is this really necessary?) immediately after loading them.
That means when __sleep() runs $vars = get_object_vars($this);, $vars['pluginCollection'] is NULL. After that we call getPluginCollections() which initializes it, but in $vars, the property is still null. Then we compare the object against the values in $vars, it's not matching anything and we're not removing it.
The extra sad part is that the whole thing is unnecessary since the object wasn't instantiated in the first place.
Proposed resolution
As a quickfix, first get the plugin collections and after that the $vars.
A nicer fix would be to run this logic *only* on initialized plugin collections, and completely skip it for this case. I guess this needs a method/interface that extends ObjectWithPluginCollectionInterface, or maybe this is entity specific.
Comments
Comment #2
berdirComment #3
alexpottThe approach looks like a good quick fix. I think adding a test case would be beneficial to make sure we don't break this in the future.
Comment #4
berdirWe have unit test coverage for __sleep(), but that didn't trigger this case because it did not lazy-instantiate the plugin collection. I changed it a bit now and also included test coverage to cover possibly injected services, like we have now. That's not *really* a unit test anymore, but it is testing that the parent is called properly as well.
And because unit tests, I also added a kernel test for the filter format config entity.
Comment #6
tim.plunkettReally tricky bug! The fix looks great, as does the test (and the fix to that test code).
Comment #7
berdirFollow-up: #3026171: Optimize ConfigEntityBase::__sleep(), only go through plugins if they were instantiated
Comment #8
alexpottCommitted and pushed 5fbbbde83d to 8.7.x and da56eaa0ab to 8.6.x. Thanks!