deliciousbrains
Do not use transients for locking
lock_process(), unlock_process() and is_process_running() use a transient for indicating whether the process is currently running.
I'm sure this is convenient as it allows you to specify an expiry time, helping avoid locks that get stale in the event of a job failing. However it's also unreliable since transients can disappear at any time. They're not guaranteed to persist until their expiration time which is a bad trait for a lock that's supposed to prevent double processing.
I'd suggest using normal options instead
Indeed, we are seeing twice executed tasks using this library. Rarely but such race conditions happen.
i keep running into my tasks being executed twice. did either of you find a stable solution?
The issue I reported here is only likely to cause issues in a very narrow set of circumstances, so if you're regularly having tasks run twice then this is (probably) not the problem
In our case there are about 20 background tasks per day and we got every week some duplicate runs. In the meantime we moved the business logic to an external Node.js server and detect duplicate calls there. Just a workaround, not a solution.
My guess would be that you're queueing up multiple jobs rather than it incorrectly running the same job twice.
In which case I'd suggest opening a fresh ticket about that, it's unlikely that the issue in this thread is responsible.
We analyzed your code and the transients are the root cause. There are clearly race conditions and the WordPress developers themselves write in their documentation that transients are not a safe way.
Erm, it's not my code.... I just reported this issue on finding that transients were being used for locking.
My report was purely hypothetical, so if you've investigated this and determined that it's definitely transients then perhaps posting the details of your investigation will help the package developers when they look at this.
ultimately i just decided to do cleanup during the job process itself to make sure duplicate actions wont be taken. from the debug logging i did it appears the cron healthcheck was never able to verify the transient (i also tried normal options and it wasnt able to update the option lock during the cron healthcheck). so maybe the cron check on my implementation doesnt have the permissions necessary to update the options? it's weird because it definitely had permissions to insert posts...
anyways ive got a solution for me that while doesnt prevent the double jobs running from the cron check (the is_process_locked() would always fail to prevent the double job in the cron healthcheck) i get my right end result.
I recommend using file system locking or an object cache with such a feature. With multiple parallel running PHP processes this is still an issue and not solved by closing this issue.
I'd echo @cbratschi's comments. The original issue I reported (the incorrect use of transients for locking) is still valid, and should still be resolved.