Contrib.social

Entity save post transactional hook

Open on Drupal.org →
Created on 6 July 2017, almost 7 years ago
Updated 26 March 2023, over 1 year ago

Problem/Motivation

Currently if a DB supporting transactions is used then on entity save the transaction will be committed after the SqlContentEntityStorage::save() method is finished and the transaction object is out of scope so that its destructor is executed. This means that all the hooks will be fired before the transaction is committed including the update hook.

If you are building a system where a lock for the entity is being created in the presave hook and should be released after the entity is saved then using the update hook while in transaction might lead to race conditions e.g. by releasing the lock and a concurrent request trying to get a lock and load the entity with the expectation that the entity will be the newly saved entity by the other process. However this will not be the case if it happens that the lock is retrieved by the concurrent process just before the transaction is committed in which case the concurrent process will load the entity in its previous state.

In order to overcome this we need a hook that will be fired after the transaction is committed.

Proposed resolution

In SqlContentEntityStorage::save() commit the transaction and start a new one inside which invoke an entity post transactional hook.

Remaining tasks

Decide between #25 and #33 for proposed solution
Add tests
Review

User interface changes

API changes

Data model changes

✨ Feature request
Status

Needs work

Version

10.1 ✨

Component
Entity  →

Last updated 12 minutes ago

  • Maintained by
  • 🇬🇧United Kingdom @catch
  • 🇨🇭Switzerland @Berdir
  • 🇩🇪Germany @hchonov
Created by

🇩🇪Germany hchonov 🇪🇺🇩🇪🇧🇬

Live updates comments and jobs are added and updated live.
  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇺🇦Ukraine Mykola Dolynskyi Poltava

    Core firing hook_entity_insert() and hook_entity_insert() before transaction is finished may be not so bad, but bad there are no same hooks to be fired after transaction finished and no tables locked!

  • First commit to issue fork.
  • Status changed to Needs review over 1 year ago
  • Comment over 1 year ago →
    🇮🇳India kunal_sahu Karnataka

    The above patch is committing the transaction and then starting a new one inside the if block that checks if the parent save() method has returned true. This ensures that the post transactional hook is only invoked if the entity save operation was successful.
    The moduleHandler is used to invoke the entity_post_transactional_save hook, which should be implemented by any modules that need to perform actions that are not part of the main entity save transaction.

    Please review. Thanks

  • Status changed to Needs work over 1 year ago
  • Comment over 1 year ago →
    🇺🇸United States smustgrave

    @kunal_sahu can you explain why your solution is a better approach then the previous patches please.

    Added to the remaining tasks decision needs to be made between #23 and #33.

    Test coverage will be needed either wya.

contrib.social Blog FAQ Discussions
Production build 0.69.0 2024