[11.x] Fix unique job lock is not released on model not found exception, lock gets stuck.

[zackAJ]

Description

When using SerializesModels on a Unique job and the model is deleted before the job is being processed, ModelNotFoundException will be thrown and the job will fail and that's okay and well documented.
However the unique lock is not released and gets stuck until expired, this happens a lot with delayed jobs.

Related issues: #50211, #49890, possibly #37729

Steps to reproduce

1- Create or open a laravel project, for ease of inspecting use database cache driver.

2- Create TestJob.php in your app/Jobs :

<?php

namespace App\Jobs;

use App\Models\User;
use Illuminate\Contracts\Queue\ShouldBeUnique;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;

class TestJob implements ShouldQueue, ShouldBeUnique
{
    use InteractsWithQueue, Dispatchable, SerializesModels;

    public function __construct(public User $user) {}

    public function handle()
    {
    }

    public function uniqueId(): string
    {
        return "some_key";
    }
}

3- Add a test route to your web.php:

<?php

use App\Jobs\TestJob;
use App\Models\User;
use Illuminate\Support\Facades\Route;

Route::get('test', function () {
    /** @var \App\Models\User */
    $user = User::factory()->create();

    dispatch(new TestJob($user));

    $user->delete();

    return "job will fail and lock will be stuck, if you refresh the page it won't dispatch again";
});

4- Run the web and queue servers via composer run dev and hit the /test route

5- Refresh the page and inspect your database's cache_locks and failed_jobs tables

Cause

TLDR

No serialized job command instance = no unique lock to release

    /**
     * Ensure the lock for a unique job is released.
     *
     * @param  mixed  $command
     * @return void
     */
    protected function ensureUniqueJobLockIsReleased($command)
    {
        if ($command instanceof ShouldBeUnique) {
            (new UniqueLock($this->container->make(Cache::class)))->release($command);
        }
    }

In depth

callQueuedHanlder.php's call() is responsible for handling the queued job, when we try to unserialize the payload of the job ModelNotFoundException is thrown because the model was deleted.

In this case we don't have a job command instance which is currently required to release the lock.

    /**
     * Handle the queued job.
     *
     * @param  \Illuminate\Contracts\Queue\Job  $job
     * @param  array  $data
     * @return void
     */
    public function call(Job $job, array $data)
    {
        try {
            $command = $this->setJobInstanceIfNecessary(
                $job, $this->getCommand($data) //this fails
            );
        } catch (ModelNotFoundException $e) {
            return $this->handleModelNotFound($job, $e); //early return: this will call $job->fail() eventually triggering failed()
        }
        
      // unreachable code (depends on a job command instance and releases the lock)
      if ($command instanceof ShouldBeUniqueUntilProcessing) {
            $this->ensureUniqueJobLockIsReleased($command);
      }
      ...
    }

after $this->handleMedelNotFound(...) is done the job is marked as failed and the failed() method will be called:

    /**
     * Call the failed method on the job instance.
     *
     * The exception that caused the failure will be passed.
     *
     * @param  array  $data
     * @param  \Throwable|null  $e
     * @param  string  $uuid
     * @return void
     */
    public function failed(array $data, $e, string $uuid)
    {
        $command = $this->getCommand($data); // this will fail again
        
        // unreachable code (depends on a job command instance and releases the lock)
        
        if (! $command instanceof ShouldBeUniqueUntilProcessing) { 
            $this->ensureUniqueJobLockIsReleased($command);
        }
        ...
     }

Solution

The idea

Wrap the job with a hidden context in order to have access to the lock key and the cache driver used, here's a simple overview of the flow:

context()->addHidden([
    'lock_key' => $lockKey,
    'cache_driver' => $cacheDriver
]);

dispatch(new TestJob($user));

context()->forgetHidden(['lock_key','cache_driver']);

Now when handling ModelNotFoundException via the handleModelNotFound() method, we can forceRelease the lock by using the provided cache_driver and lock_key from the hidden context

        $driver = context()->getHidden('cache_driver');

        $key = context()->getHidden('lock_key');

        if ($driver && $key) {
            cache()->driver($driver)->lock($key)->forceRelease();
        }

Implementation

See code diff.

If this gets closed for any reason use my test to research and implement another possible solution, add this to tests/Integration/Queue/UniqueTestJob.php

    use Orchestra\Testbench\Factories\UserFactory;

    public function testLockIsReleasedOnModelNotFoundException()
    {
        UniqueTestSerializesModelsJob::$handled = false;

        /**  @var  \Illuminate\Foundation\Auth\User */
        $user = UserFactory::new()->create();
        $job = new UniqueTestSerializesModelsJob($user);

        $this->expectException(ModelNotFoundException::class);

        try {
            $user->delete();
            dispatch($job);
            $this->runQueueWorkerCommand(['--once' => true]);
            unserialize(serialize($job));
        } finally {
            $this->assertFalse($job::$handled);
            $this->assertModelMissing($user);
            $this->assertTrue($this->app->get(Cache::class)->lock($this->getLockKey($job), 10)->get());
        }
    }


class UniqueTestSerializesModelsJob extends UniqueTestJob
{
    use SerializesModels;

    public function __construct(public User $user) {}
}

[zackAJ]

I'll handle the 5 failing checks

[zackAJ]

the test is passing locally but failing in CI check ?

[zackAJ]

I'll handle the 5 failing checks

done

[taylorotwell]

I'm curious - is the lock released if you use this: https://laravel.com/docs/11.x/queues#ignoring-missing-models

[zackAJ]

I'm curious - is the lock released if you use this: https://laravel.com/docs/11.x/queues#ignoring-missing-models

nope it's not, I've updated the test btw to run correctly both locally and in CI.

[zackAJ]

@taylorotwell any comments on this ?

[taylorotwell]

Thanks!

[zackAJ]

SAHA 🇩🇿

[macropay-solutions]

@zackAJ

If this gets closed for any reason use my test to research and implement another possible solution, add this to tests/Integration/Queue/UniqueTestJob.php

What do you think of putting the
laravel_unique_job_cache_store
and
laravel_unique_job_key
into the job's payload to avoid the context and to cover also Queue::push(new JobClass($model)); facade usage scenario

Update.
The payload can be added to the payoad via

\Illuminate\Queue\Queue::createPayload:

\Illuminate\Queue\CallQueuedHandler::handleModelNotFound:

Also it might be worth documenting that Queue::push facade should not be used for dispatching ShouldBeUnique jobs