Resetting an entity_load cache bin in Drupal 7 can have more drastic consequences than in previous versions of Drupal, as we can have persistent load caches in place of the PHP static memory cache.
Instead, wherever possible, only flush the necessary entities from the cache, using entity_get_controller($entity_type)->resetCache(array($entity_id));
In this case, the use of $reset in workbench_moderation_node_live_load() appears to be entirely redundant, as a revision id is being specified, and when loading a revision the entity load cache is not used at all (even if it is the current revision); the entity object is always generated freshly in that situation.
As such, I think we can simply remove the flag from that call.
n.b. Within this module, workbench_moderation_node_live_load() is only called by workbench_moderation_store(), which in turn is only called by workbench_moderation_moderate() via drupal_register_shutdown_function('workbench_moderation_store', $node).
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | state_machine-entity_load_cache_reset-1963992-9.patch | 866 bytes | jweowu |
Comments
Comment #1
jweowu commentedComment #2
jweowu commentedAnd because I need it at the moment (and in case anyone else does too), here's a version of that patch which applies to version 1.2 of the module.
Comment #2.0
jweowu commentedCross-referencing with core API issue
Comment #3
acbramley commentedI've been using this patch for a long time on multiple projects with no side affects, marking RTBC
Comment #4
recrit commentedre-roll for latest 7.x-1.x
Comment #5
colanThanks!
Comment #7
acbramley commentedThere's only a single node_load call in workbench_moderation on the 7.x-2.x branch, I think that means that the functionality this patch fixes would now be in state machine therefore we don't need to port a patch, but potentially open an issue there?
Comment #8
colanMoved to the other queue. Close if not an issue.
Comment #9
jweowu commentedYes, a grep of the code shows that this is also an issue here.
It's probably the same fix -- it's a revision which is being loaded in each case here, so simply removing the
$reset = TRUEargument has no effect on the return values of thosenode_load()calls.We need to confirm whether the other code is making assumptions about the rest of the entity load cache, however. If anything needs to be purged from the load cache, we need to do that manually with
entity_get_controller($entity_type)->resetCache(array($entity_id));I've not looked at state_machine before, so would appreciate input from someone who knows the code.
Comment #10
jweowu commentedThat patch is actually for the
7.x-2.xbranch, and I can confirm that the bug doesn't exist in7.x-3.xComment #11
jweowu commentedComment #12
jweowu commented