Issue #2939491 by hchonov, slasher13, alexpott, dawehner: Prevent memory leak and inconsistent entity references when serializing entity storages

core/lib/Drupal/Core/DependencyInjection/DependencySerializationTrait.php

@@ -2,6 +2,7 @@
 
 namespace Drupal\Core\DependencyInjection;
 
+use Drupal\Core\Entity\EntityStorageInterface;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 
 /**
@@ -16,6 +17,13 @@ trait DependencySerializationTrait {
    */
   protected $_serviceIds = [];
 
+  /**
+   * An array of entity type IDs keyed by the property name of their storages.
+   *
+   * @var array
+   */
+  protected $_entityStorages = [];
+
   /**
    * {@inheritdoc}
    */
@@ -35,6 +43,17 @@ trait DependencySerializationTrait {
         $this->_serviceIds[$key] = 'service_container';
         unset($vars[$key]);
       }
+      elseif ($value instanceof EntityStorageInterface) {
+        // If a class member is an entity storage, only store the entity type ID
+        // the storage is for so it can be used to get a fresh object on
+        // unserialization. By doing this we prevent possible memory leaks when
+        // the storage is serialized when it contains a static cache of entity
+        // objects and additionally we ensure that we'll not have multiple
+        // storage objects for the same entity type and therefore prevent
+        // returning different references for the same entity.
+        $this->_entityStorages[$key] = $value->getEntityTypeId();
+        unset($vars[$key]);
+      }
     }
 
     return array_keys($vars);
@@ -61,6 +80,19 @@ trait DependencySerializationTrait {
       $this->$key = $container->get($service_id);
     }
     $this->_serviceIds = [];
+
+    // In rare cases, when test data is serialized in the parent process, there
+    // is a service container but it doesn't contain all expected services. To
+    // avoid fatal errors during the wrap-up of failing tests, we check for this
+    // case, too.
+    if ($this->_entityStorages && (!$phpunit_bootstrap || $container->has('entity_type.manager'))) {
+      /** @var \Drupal\Core\Entity\EntityTypeManagerInterface $entity_type_manager */
+      $entity_type_manager = $container->get('entity_type.manager');
+      foreach ($this->_entityStorages as $key => $entity_type_id) {
+        $this->$key = $entity_type_manager->getStorage($entity_type_id);
+      }
+    }
+    $this->_entityStorages = [];
   }
 
 }

core/lib/Drupal/Core/Entity/EntityStorageBase.php

@@ -541,4 +541,16 @@ abstract class EntityStorageBase extends EntityHandlerBase implements EntityStor
    */
   abstract protected function getQueryServiceName();
 
+  /**
+   * {@inheritdoc}
+   */
+  public function __sleep() {
+    // In case the storage is being serialized then we prevent from serializing
+    // the static cache of entities together with it, as this could lead to a
+    // memory leak.
+    $vars = parent::__sleep();
+    unset($vars['entities']);
+    return $vars;
+  }
+
 }

core/tests/Drupal/KernelTests/Core/Entity/ContentEntityCloneTest.php

@@ -297,7 +297,7 @@ class ContentEntityCloneTest extends EntityKernelTestBase {
     // Retrieve the entity properties.
     $reflection = new \ReflectionClass($entity);
     $properties = $reflection->getProperties(~\ReflectionProperty::IS_STATIC);
-    $translation_unique_properties = ['activeLangcode', 'translationInitialize', 'fieldDefinitions', 'languages', 'langcodeKey', 'defaultLangcode', 'defaultLangcodeKey', 'revisionTranslationAffectedKey', 'validated', 'validationRequired', 'entityTypeId', 'typedData', 'cacheContexts', 'cacheTags', 'cacheMaxAge', '_serviceIds'];
+    $translation_unique_properties = ['activeLangcode', 'translationInitialize', 'fieldDefinitions', 'languages', 'langcodeKey', 'defaultLangcode', 'defaultLangcodeKey', 'revisionTranslationAffectedKey', 'validated', 'validationRequired', 'entityTypeId', 'typedData', 'cacheContexts', 'cacheTags', 'cacheMaxAge', '_serviceIds', '_entityStorages'];
 
     foreach ($properties as $property) {
       // Modify each entity property on the clone and assert that the change is