Infinite recursion in action deriver

Created on 6 June 2024, 5 months ago
Updated 26 August 2024, 3 months ago

Problem/Motivation

There is a possible infinite recursion in \Drupal\eca_vbo\Plugin\Action\Derivative\VboExecuteDeriver::getDerivativeDefinitions. I'm not able to reproduce this deliberately, but I experience that almost every other day in the development environment. It may happen when both state and plugin cache need to be rebuilt at the same time. A call to \Drupal::entityTypeManager()->getStorage('eca')->loadMultiple() also needs to initialize all action plugins, which causes this deriver to also call the load multiple eca models - and there we go with the recursion.

Proposed resolution

Prevent recursion in that deriver.

πŸ› Bug report
Status

Fixed

Version

2.1

Component

Code

Created by

πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @jurgenhaas
  • Status changed to Fixed 5 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
  • Automatically closed - issue fixed for 2 weeks with no activity.

  • πŸ‡ΊπŸ‡ΈUnited States Glottus

    I'm not sure this is completely fixed.

    Building out a new D10 site (currently 10.3.1) where we use Webform as a per-entity registration form and ECA to handle various automated tasks, we got to a stage where we want to start building models that can get triggered in bulk when viewing a the webform submissions attached to a node.

    This site is still very much in early stages with fewer than 10 such event nodes and/or webform submissions to list, and there are only 6 or 7 ECA models built; just one single model and one single View which implement the VBO functionality so far.

    After adding and enabling the vbo and eca_vbo modules (2.0.0), we immediately began noticing resource issues, mainly "allowed memory size... exhausted". This is true on both local development (DDEV) and test server environments where we tried increasing settings such as "memory_limit" in our .user.ini file, but with not much improvement. Occasionally pages will fail with a white screen and short message to help, and finally one of the messages appeared with more detail:

    ECA ran into error from third party in the context of "Collecting all available actions": Allowed memory size of 536870912 bytes exhausted (tried to allocate 126976 bytes) Line 113 of /var/www/html/web/core/lib/Drupal/Core/Database/StatementWrapperIterator.php

    To attempt some basic troubleshooting, I modified the /eca_vbo/src/Plugin/Action/Derivative/VboExecuteDeriver.php file to wrap one of the foreach loops with an artificial limit:

            $i = 100;
            while ($i > 0) {
              foreach (($eca->get('events') ?? []) as $event) {
                if (!in_array($event['plugin'], $plugin_ids, TRUE)) {
                  continue;
                }
                  $operation_name = $event['configuration']['operation_name'] ?? '';
                  $action_id = strtolower(preg_replace("/[^a-zA-Z0-9]+/", "_", trim($operation_name)));
                  if ($action_id !== '' && $action_id !== '_') {
                    if (!isset($this->definitions[$action_id])) {
                      $this->definitions[$action_id] = [
                        'label' => str_replace('_', ' ', ucfirst($operation_name)),
                        'operation_name' => $operation_name,
                      ] + $base_plugin_definition;
                    }
                    $this->definitions[$action_id]['eca_config'][] = $eca->id();
                  }
              }
              $i--;
            }
    

    Not knowing exactly how this is SUPPOSED to exit cleanly, I wanted to see if functionality returned, and it did. Without the artificial limit above, I quickly get errors that begin like the following trace:

    The website encountered an unexpected error. Try again later.
    Error: Xdebug has detected a possible infinite loop, and aborted your script with a stack depth of '1000' frames in Drupal\Component\DependencyInjection\Container->hasParameter() (line 342 of core/lib/Drupal/Component/DependencyInjection/Container.php).
    Drupal\Component\DependencyInjection\Container->get('string_translation') (Line: 197)
    Drupal::service('string_translation') (Line: 215)
    Drupal\Core\StringTranslation\TranslatableMarkup->getStringTranslation() (Line: 190)
    Drupal\Core\StringTranslation\TranslatableMarkup->render() (Line: 15)
    Drupal\Core\StringTranslation\TranslatableMarkup->__toString()
    sprintf('%s %s', Object, Object) (Line: 70)
    Drupal\Core\Action\Plugin\Action\Derivative\EntityActionDeriverBase->getDerivativeDefinitions(Array) (Line: 101)
    Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDerivatives(Array) (Line: 87)
    Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDefinitions() (Line: 337)
    Drupal\Core\Plugin\DefaultPluginManager->findDefinitions() (Line: 213)
    Drupal\Core\Plugin\DefaultPluginManager->getDefinitions() (Line: 22)
    Drupal\Core\Plugin\DefaultPluginManager->getDefinition('entity:delete_action:media') (Line: 16)
    Drupal\Core\Plugin\Factory\ContainerFactory->createInstance('entity:delete_action:media', Array) (Line: 83)
    Drupal\Component\Plugin\PluginManagerBase->createInstance('entity:delete_action:media', Array) (Line: 178)
    Drupal\eca\PluginManager\Action->createInstance('entity:delete_action:media', Array) (Line: 62)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->initializePlugin('entity:delete_action:media') (Line: 80)
    Drupal\Component\Plugin\LazyPluginCollection->get('entity:delete_action:media') (Line: 18)
    Drupal\Core\Action\ActionPluginCollection->get('entity:delete_action:media') (Line: 88)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->setConfiguration(Array) (Line: 104)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->addInstanceId('entity:delete_action:media', Array) (Line: 55)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->__construct(Object, 'entity:delete_action:media', Array) (Line: 114)
    Drupal\system\Entity\Action->getPluginCollection() (Line: 130)
    Drupal\system\Entity\Action->getPlugin() (Line: 145)
    Drupal\system\Entity\Action->getPluginDefinition() (Line: 72)
    Drupal\eca\Plugin\Action\PreConfiguredActionDeriver->getDerivativeDefinitions(Array) (Line: 101)
    Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDerivatives(Array) (Line: 87)
    Drupal\Component\Plugin\Discovery\DerivativeDiscoveryDecorator->getDefinitions() (Line: 337)
    Drupal\Core\Plugin\DefaultPluginManager->findDefinitions() (Line: 213)
    Drupal\Core\Plugin\DefaultPluginManager->getDefinitions() (Line: 22)
    Drupal\Core\Plugin\DefaultPluginManager->getDefinition('action_message_action') (Line: 16)
    Drupal\Core\Plugin\Factory\ContainerFactory->createInstance('action_message_action', Array) (Line: 83)
    Drupal\Component\Plugin\PluginManagerBase->createInstance('action_message_action', Array) (Line: 62)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->initializePlugin('action_message_action') (Line: 80)
    Drupal\Component\Plugin\LazyPluginCollection->get('action_message_action') (Line: 88)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->setConfiguration(Array) (Line: 104)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->addInstanceId('action_message_action', Array) (Line: 55)
    Drupal\Core\Plugin\DefaultSingleLazyPluginCollection->__construct(Object, 'action_message_action', Array) (Line: 871)
    Drupal\eca\Entity\Eca->getPluginCollections() (Line: 165)
    Drupal\Core\Config\Entity\ConfigEntityBase->set('weight', 0) (Line: 163)
    Drupal\eca\Entity\Eca::postLoad(Object, Array) (Line: 389)
    Drupal\Core\Entity\EntityStorageBase->postLoad(Array) (Line: 319)
    Drupal\Core\Entity\EntityStorageBase->loadMultiple() (Line: 56)

  • Status changed to Needs work 4 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen
  • Status changed to Needs review 4 months ago
  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    I think I got it: the variable to collect the plugin definitions needs to be static to catch all the scenarios that can lead to the infinite loop. Please review and test if that fixes your scenario as well.

  • πŸ‡ΊπŸ‡ΈUnited States Glottus

    So far, so good! I've opened another issue that has come up now, but I don't know if it is related to these specific changes. The good news is that by using the adjustments to static variables as in your MR, I have been able to move forward without re-experiencing the resources issues that had been plaguing this project after enabling eca_vbo.

    The new issue is here: https://www.drupal.org/project/eca_vbo/issues/3464059 πŸ› Edits to model appear to break configuration in View Active

  • Status changed to Needs work 4 months ago
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    It may happen when both state and plugin cache need to be rebuilt at the same time.

    Could you please tell what is a "state plugin"?

    I'm not yet convinced this is the right solution. Before continuing on this, we need a test that will reproduce this. Once we have a proper test that reproduces this correctly, then we can try to find a proper solution. Otherwise this is just a guess game.

    A second indicator this is not the solution yet is the reported problem in πŸ› Edits to model appear to break configuration in View Active which is the first time I see that the problem got reported. But this module is around for quite some time now, with more than 100 site installations. Some other user would already have gotten to the same problem and reported it already. But now you have the problem and reported it at the time you tried the current MR of this issue.

    Besides that, please don't create new issues already that are coming from other issues that are not yet fixed.

    At least for now, we need to work towards a proper test. If you don't know how to do it no problem, maybe you can at least provide a configuration export (no whole site export, just the relevant ones) together with a minimal list of steps, that will reproduce your problem.

  • Issue was unassigned.
  • Status changed to Needs review 4 months ago
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    I'm able to reproduce the infinite recursion, also with the latest branch of 2.1.x that contains the first fix attempt of this issue.

    Please have a look first whether the infinite recursion is fixed for you with merge request !3. Let me know whether that fixes the problem for your or not. Setting to "Needs review" for that one - no matter of the result we still need proper test coverage on this particular problem.

    The actual part which is triggering the infinite recursion is here: https://git.drupalcode.org/project/eca/-/blob/2.1.x/src/Entity/Eca.php?r...
    That part is extremely bad resource-wise, because it's already loading all plugins when just loading ECA config entities. This must be fixed as well, but it's not part of this issue (belongs into a new issue within the ECA issue queue). It can easily be seen when debugging the recursion with Xdebug. Next time please provide a stack trace / debug stack so we may have a chance to identify this earlier.

    @jurgenhaas I appreciate your efforts on working on this module. Let's please get through a proper review process next time, instead of directly committing and setting to fixed without merge requests. We could also work with a "timer", let's say after two weeks of no response someone of us may proceed at their own will. For a minimum I'd like to get a chance for reviewing first.

  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    Attaching a sample model for a VBO process that works after applying MR !3.

    There are some aspects that may sometimes be overseen easily:

    • You need to use the "Global: ECA bulk operations" views field plugin within your configured view.
    • You need to use the action "VBO: Set custom access on Views Bulk Operation" after the event "VBO: Custom access for Views bulk operation" for granting access.

    Probably some more hurdles, I know it's not that easy to set it up correctly.

  • πŸ‡ΊπŸ‡ΈUnited States Glottus

    Thank you, mxh, my ECA model does include the "VBO: Custom access for Views bulk operation" (I've done this with several other models on other sites successfully) and my view does include the "Global: ECA bulk operations" field". The proposed code edits by jurgenhaas allowed me to move forward on development for now, but I'll continue to test the other issue that I opened, as even with those pieces above, I see the bug of the disappearing checkboxes every once in a while. I'll report back more on that thread if I can offer anything more.

  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    @Glottus thanks for your response - have you tried merge request !3 already, or do you mean with "those pieces above" the hints from #13?

    When MR3 doesn't work either, feel free to directly set this back to "Needs work".

  • πŸ‡©πŸ‡ͺGermany jurgenhaas Gottmadingen

    Thanks for all the great collaboration in here. Glad to see this moving forward.

    When it comes to a test, here is how I can manually reproduce this: Installing a fresh Drupal 10.3.1 site, enabling eca_vbo (and all the dependencies) and then rebuilding the cache with drush cr. The next page request for an authenticated user runs into the issue. When I interrupt that request and submit a new one, then the page loads correctly.

    Not sure how something like that could be built into a test.

    Let's please get through a proper review process next time, instead of directly committing and setting to fixed without merge requests. We could also work with a "timer", let's say after two weeks of no response someone of us may proceed at their own will. For a minimum I'd like to get a chance for reviewing first.

    Agreed!

  • πŸ‡ΊπŸ‡ΈUnited States Glottus

    @mxh - I have not tried MR !3 yet, no.

    Only MR !2, but with that, plus your hints from #13 already in place, I was getting the issue I reported separately.

    I'll be able to try MR !3 next week. That should be done as a separate test from the changes in MR !2 (reverting those changes first), correct?

  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    @Glottus yes, MR !3 from branch eca_vbo-3452900/3452900-possible-infinite-recoursion--2 needs to be tried out separately from MR !2.

  • πŸ‡ΊπŸ‡ΈUnited States Glottus

    @mxh - this may not be very helpful, but when I switch from applying MR !2 as a patch to MR !3, performance issues come back. I'm not sure I can reliably recreate the issue, but as @jurgenhaas described, it seems that pages (especially one where the View implementing eca_vbo functionality) take an excessive amount of time most often just after a cache clear. When I switch back to MR !2 instead, that problem disappears and everything appears to work well. No stack traces or watchdog entries to help, unfortunately.

    Too early to say if my other issue (the disappearing checkboxes) remains with either of those patches because THAT problem was only intermittent, as well, and in the process of developing my model (using MR !2), I discovered that I was in fact overthinking my Model workflow, and that I could remove the "Entity Load" Action that I had been testing, so for the other issue I've opened, there may be something completely different going on having to do with my own assumptions and choices there.

    For now, I'll keep MR !2 in place to continue building out the functionality we need, but am hoping for a stable solution to be found soon. Thanks for your help on this!

  • Assigned to mxh
  • Status changed to Needs work 4 months ago
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    @Glottus thanks for your feedback on this. Don't know what might be the issue on the performance. It's not supposed to run into that deriver method multiple times on runtime when cache is working properly. Given the input from you and @jurgenhaas in this issue, I'll try to write a test that can at least reproduce the infinite recursion and then will see what may be a proper solution for it.

  • πŸ‡©πŸ‡ͺGermany mxh Offenburg
    • mxh β†’ committed a08fbefe on 2.1.x
      Issue #3452900 by mxh, jurgenhaas, Glottus: Infinite recursion in action...
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    I've written a simple test that reproduces the recursion. I've committed it into the current branches and MRs with following results:

    • origin/2.1.x the test reproduces the infinite recursion
    • eca_vbo-3452900/'3452900-possible-infinite-recoursion' (MR !2) the test fails because the action definition is not available
    • eca_vbo-3452900/'3452900-possible-infinite-recoursion--2' (MR !3) the tests passes as the action definition is available and the recursion is not happening

    As the test proves that MR !3 is resolving the recursion problem, I'll continue with that one and see what else can be optimized in hope to further reduce potential performance problems.

  • Issue was unassigned.
  • Status changed to Needs review 4 months ago
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    I've added a memory cache layer for the deriver in hope that it will be more efficient when being called multiple times on the same request (if for any reason).

    Setting to Needs Review for MR !3 once again.

    I fear that #19 could not be addressed here, but at the same time I currently believe that the performance problem reported there is originating from something else. For being able to track down the performance problem reported, more details are needed. Best would be to create a sample configuration based on Drupal's standard installation profile. Until now I was not able to reproduce that on a standard installation profile.

  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    mxh β†’ changed the visibility of the branch 3452900-possible-infinite-recoursion to hidden.

  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    I've set the visibility of MR 2 to hidden so that it won't be confused on reviewing. That MR can still be found here.

    • mxh β†’ committed a695a42c on 2.1.x
      Issue #3452900 by jurgenhaas, Glottus: Possible infinite recursion in...
  • Status changed to Fixed 4 months ago
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    I'm sorry but I had to merge this one for now as it fixes the recursion problem on our sites. Please report back in case you still encounter a problem with that. When it's about being slow but not running into infinite recursion, then we most probably face a different problem that should be handled in a separate issue. However, when you still encounter memory exhaustion because of infinite recursion, this issue needs further work.

    • mxh β†’ committed a695a42c on 2.0.x
      Issue #3452900 by jurgenhaas, Glottus: Possible infinite recursion in...
    • mxh β†’ committed a08fbefe on 2.0.x
      Issue #3452900 by mxh, jurgenhaas, Glottus: Infinite recursion in action...
  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    Interestingly enough, the infinite recursion does not occur on ECA 1.x and eca_vbo 1.x. Therefore backported to 2.0.x only.

  • πŸ‡ΊπŸ‡ΈUnited States Glottus

    Thanks for all the recent work on this. Juggling multiple projects, but just today implemented your newest release (2.0.1) and so far, everything is working well - though at this point, that means that other tasks are all running smoothly: clearing cache, running update.php, etc. I don't have more work to do on the primary project that has to do with ECA and/or eca_vbo, but this is promising.

    Promising enough that I also updated to 2.0.1 on a separate project with many more eca_vbo-related ECA Models, but THAT site had still been on ECA 1.x and eca_vbo 1.x, and as you said, had never experienced any such issues (not the performance issues with infinite recursion, nor the other issue I opened about disappearing checkboxes). I implemented the update along with everything else to get up to Drupal 10.3.1 and will see how that goes, but again, promising!

    I'll keep an eye on any other issues and report back, but the next two weeks include a lot of away-from-keyboard time, so I might not be able to have much else soon, except to say that my colleague - while downloading our latest code base with MR !2 still implemented above - also experienced the disappearing eca_vbo checkboxes in the site that only has one such view, but as I've noticed, only re-saving the related ECA Model cleared that up on the next reload. We'll see if it recurs with the latest commit.

    If nobody else reports having the original infinite recursion issue here, then hopefully this can be closed!

  • πŸ‡©πŸ‡ͺGermany mxh Offenburg

    Thanks @Glottus for your reply. You can of course report back anytime when you or your colleague notice something. Hope you can enjoy the time being away from the keyboard.

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

Production build 0.71.5 2024