Problem/Motivation
If you set up a field to use queue, there is a race condition where the field can be edited before cron runs.
Steps to reproduce
- set up a field to use queue
- create a node and set the field to refer to taxonomy term 1
- delete taxonomy term 1, which queues deletion of the node's delta 0 item
- edit the node so that the delta 0 item now refers to taxonomy term 2
- run cron
- observe that the node no longer refers to taxonomy term 2
Proposed resolution
When queueing a field item for deletion, make note of its target id. When processing the queue, check the field item's target id, to make sure it hasn't changed since the item was queued for deletion.
User interface changes
None.
API changes
The signature of entity_reference_purger_add_to_queue() changes, but only to add an optional $target_id argument, so BC is preserved.
Data model changes
The queue item structure changes, again to add an optional 'target_id' item.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 3346341-race-condition-7.patch | 10.18 KB | dgroene |
| #6 | interdiff_2-6.txt | 2.49 KB | mrshowerman |
| #6 | 3346341-race-condition-6.patch | 9.06 KB | mrshowerman |
| #2 | 3346341-race-condition.patch | 8.54 KB | ksenzee |
| #2 | 3346341-race-condition-TEST-ONLY-FAIL.patch | 5.28 KB | ksenzee |
Comments
Comment #2
ksenzeeHere's a patch that fixes the issue, with tests.
Comment #4
dalinSetting to critical since this results in unexpected data loss.
Question about the patch: why skip if the entity ID is no longer is in the expected position? Shouldn't we search for the expected ID and remove it regardless of its position? We could use
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21TypedData...
Comment #5
dalinComment #6
mrshowermanAddressing #4 while trying to stay backwards compatible (i.e., process queue items without target ID using the "old way").
Also adding a sanity check from my patch in #3348326: Exceptions are not being caught.
Comment #7
dgroene commentedRerolled for version beta5 and accounting for revisions.
Comment #8
gnikolovskiComment #10
gnikolovskiFixed.