It's possible to call scheduler_cron() simultaneously in parallel processes (e.g. during a regularly scheduled core cron run set to execute on the hour, and a once-a-minute cron calling the scheduler_cron function). When this happens without a locking mechanism in place, undesirable results can happen as both processes update the records while changes are being made (e.g. content set to Archived state while still set to published).

Adding a lock_acquire() to the top of the scheduler_cron() function fixes this issue neatly.

CommentFileSizeAuthor
#1 2534796-parallel-processing-causing-munged-status.patch1.21 KBemanaton
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

emanaton’s picture

emanaton CreditAttribution: emanaton commented
FileSize
2534796-parallel-processing-causing-munged-status.patch1.21 KB

Aaand here's a patch. =o)

jonathan1055’s picture

jonathan1055 CreditAttribution: jonathan1055 commented
Version: 7.x-1.3 » 8.x-1.x-dev

Hi emanton,

That's an intersting point. Yes I can see it could happen. Thanks for the patch - looks like a nice simple solution. So if a lock cannot be acquired would the cron function just finish without doing any more scheduler processing? I suppose that is fine because it means that another scheduler_cron is doing work at that very moment.

I've moved this to the 8.x issue queue so that we can make sure to correct it there first. Do you know if the same (or similar) lock_acquire and lock_release functions are available in 8.x?

Jonathan

btw if you had set the version to 7.x-1.x-dev then your patch would have been checked against the automated testing. I think as you set it to 7.x-1.3 it did not get queued. Not that we have any tests to check this locking yet, but it does verify the patch format and applicability, etc.

pfrenssen’s picture

pfrenssen
Location Sofia
CreditAttribution: pfrenssen commented
Status: Active » Closed (won't fix)

Since this is common functionality needed for all cron jobs this should be handled in the cron queue manager. It doesn't make any sense to put the burden of implementing this in the cron hook implementations themselves, unless you code to be duplicated across hundreds of projects.

In core this protection against multiple invocation of the cron jobs is already implemented. In Drupal 7 this is handled by drupal_cron_run() and in Drupal 8 in \Drupal\Core\Cron::run().

If you use a different system of invoking cron jobs (like Elysia Cron) then please file an issue against the project responsible. If you are invoking it directly, then the semaphores should be handled in your custom code.