Skip to content
Snippets Groups Projects

Use service destruction to fix the bug

Closes #3405976

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Alex Pott added 4 commits

    added 4 commits

    • d5a12841 - 1 commit from branch project:11.x
    • d5e1551a - Merge branch '11.x' into 3405976-minimal-approach
    • adb7423a - Fix unnecessary rebuilds during install
    • b6bec9e9 - Move kernel terminating in the installer to a centralised method

    Compare with previous version

  • 297 * {@inheritdoc}
    298 */
    299 public function destruct() {
    300 $manager = $this->transactionManager();
    301 if ($manager && $manager->inTransaction() && method_exists($manager, 'commitAll')) {
    302 $this->transactionManager()->commitAll();
    303 }
    304
    305 // BC layer.
    306 // @phpstan-ignore-next-line
    307 if (!empty($this->transactionLayers)) {
    308 // @phpstan-ignore-next-line
    309 $this->transactionLayers = [];
    310 // @phpstan-ignore-next-line
    311 $this->doCommit();
    312 }
    • Comment on lines +305 to +312

      Nitpick - shouldn't this run only when there's no TransactionManager available?

    • Author Maintainer

      I think this approach to BC is fine - like if there is a manager and there is something in $this->transactionLayers for whatever reason then you'd want this to happen.

    • Please register or sign in to reply
  • Alex Pott added 1 commit
  • Alex Pott added 6 commits

    added 6 commits

    Compare with previous version

  • Alex Pott added 2 commits

    added 2 commits

    • 5c24ac95 - Handle all open connections
    • 1e4ce10b - Tweaked approach and method names

    Compare with previous version

  • 127 127 return $this->stack;
    128 128 }
    129 129
    130 /**
    131 * Commits the entire transaction stack.
    132 *
    133 * @internal
    134 * This method exists only to work around a bug caused by Drupal incorrectly
    • It would be worth mentioning XDebug develop mode here to avoid a situation where someone thinks the code can be removed because they can’t reproduce the thing it works around.

      I also truely believe this isn’t a bug in Drupal - destruction ordering is well defined and reliable in normal operation - it’s just XDebug develop mode causes it to not happen and instead pushes you into end of script destruction which IS unreliable. So rather this is an incompatibility and this code aims to be more compatible with the runtime behaviours introduced by XDebug develop mode

    • Author Maintainer

      Added this detail to the information...

         * @internal
         *   This method exists only to work around a bug caused by Drupal incorrectly
         *   relying on object destruction order to commit transactions. Xdebug 3.3.0
         *   changes the order of object destruction when the develop mode is enabled.
    • Please register or sign in to reply
  • Alex Pott added 31 commits

    added 31 commits

    • 1e4ce10b...71c9b9f7 - 29 commits from branch project:11.x
    • 7d452070 - Fix drupal_rebuild() and other places where we boot the kernel and use the database
    • 737f5706 - Merge branch '11.x' into 3405976-minimal-approach

    Compare with previous version

  • Alex Pott added 2 commits

    added 2 commits

    Compare with previous version

  • Alex Pott added 1 commit

    added 1 commit

    • 812f3420 - Use variable consistently in statistics.php

    Compare with previous version

  • Alex Pott added 1 commit

    added 1 commit

    Compare with previous version

  • closed

  • Please register or sign in to reply
    Loading