Skip to content

[13.x] Don't force release a unique lock the job never acquired - #61234

Open
sulimanbenhalim wants to merge 1 commit into
laravel:13.xfrom
sulimanbenhalim:fix/unique-lock-force-release-steals-other-dispatch
Open

[13.x] Don't force release a unique lock the job never acquired#61234
sulimanbenhalim wants to merge 1 commit into
laravel:13.xfrom
sulimanbenhalim:fix/unique-lock-force-release-steals-other-dispatch

Conversation

@sulimanbenhalim

Copy link
Copy Markdown

Follow up to #60906.

UniqueLock::release() falls back to forceRelease() when a job has no owner token. That's right for a job that's actually holding the lock, but a job pushed straight onto the queue never acquires one, so on rollback it wipes out a lock a different dispatch is holding.

ProcessPodcast::dispatch();          // acquires the unique lock

DB::transaction(function () {
    Queue::push(new ProcessPodcast); // never acquires it

    throw new Exception;             // rollback fires the release callback
});

// the first job's lock is gone, so duplicates can be queued while it's still pending

Queue::push() and Queue::later() register the rollback callback through enqueueUsing(), but they don't go through PendingDispatch, which is where the lock actually gets acquired.

Same failure mode you fixed for retries in #60906, just at a different call site. A job using Queueable always records an owner when it acquires, so an empty owner means it never had the lock and has nothing to release.

I kept the check in the rollback callback instead of putting it in UniqueLock::release(). Doing it in release() would also change the processing path, and since uniqueFor defaults to 0 the lock has no expiry, so a job serialized before #60906 (no owner in its payload) would leak its lock permanently after upgrading. addCallbackForRollback is still registered either way, so the QueueConnectionTest expectations are untouched.

Test sits with the other unique lock tests and fails without the change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant