bbos
bbos

Reputation: 51

Symfony Lock does not lock when making two requests from the same browser

I want to prevent a user from making the same request two times by using the Symfony Lock component. Because now users can click on a link two times(by accident?) and duplicate entities are created. I want to use the Unique Entity Constraint which does not protect against race conditions itself.

The Symfony Lock component does not seem to work as expected. When I create a lock in the beginning of a page and open the page two times at the same time the lock can be acquired by both requests. When I open the test page in a standard and incognito browser window the second request doesn't acquire the lock. But I can't find anything in the docs about this being linked to a session. I have created a small test file in a fresh project to isolate the problem. This is using php 7.4 symfony 5.3 and the lock component

<?php

namespace App\Controller;

use Sensio\Bundle\FrameworkExtraBundle\Configuration\Template;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\Lock\LockFactory;
use Symfony\Component\Routing\Annotation\Route;

class LockTest extends AbstractController
{
    /**
     * @Route("/test")
     * @Template("lock/test.html.twig")
     */
    public function test(LockFactory $factory): array
    {
        $lock = $factory->createLock("test");

        $acquired = $lock->acquire();

        dump($lock, $acquired);

        sleep(2);

        dump($lock->isAcquired());

        return ["message" => "testing"];
    }
}

Upvotes: 5

Views: 2101

Answers (1)

I slightly rewrote your controller like this (with symfony 5.4 and php 8.1):

class LockTestController extends AbstractController
{
    #[Route("/test")]
    public function test(LockFactory $factory): JsonResponse
    {
        $lock = $factory->createLock("test");

        $t0 = microtime(true);
        $acquired = $lock->acquire(true);
        $acquireTime = microtime(true) - $t0;

        sleep(2);

        return new JsonResponse(["acquired" => $acquired, "acquireTime" => $acquireTime]);
    }
}

It waits for the lock to be released and it counts the time the controller waits for the lock to be acquired.

I ran two requests with curl against a caddy server.

curl -k 'https://localhost/test' & curl -k 'https://localhost/test'

The output confirms one request was delayed while the first one slept with the acquired lock.

{"acquired":true,"acquireTime":0.0006971359252929688}
{"acquired":true,"acquireTime":2.087146043777466}

So, the lock works to guard against concurrent requests.

If the lock is not blocking:

$acquired = $lock->acquire(false);

The output is:

{"acquired":true,"acquireTime":0.0007710456848144531}
{"acquired":false,"acquireTime":0.00048804283142089844}

Notice how the second lock is not acquired. You should use this flag to reject the user's request with an error instead of creating the duplicate entity.

If the two requests are sufficiently spaced apart to each get the lock in turn, you can check that the entity exists (because it had time to be fully committed to the db) and return an error.

Despite those encouraging results, the doc mentions this note:

Unlike other implementations, the Lock Component distinguishes lock instances even when they are created for the same resource. It means that for a given scope and resource one lock instance can be acquired multiple times. If a lock has to be used by several services, they should share the same Lock instance returned by the LockFactory::createLock method.

I understand two locks acquired by two distinct factories should not block each other. Unless the note is outdated or wrongly phrased, it seems possible to have non working locks under some circumstances. But not with the above test code.

StreamedResponse

A lock is released when it goes out of scope.

As a special case, when a StreamedResponse is returned, the lock goes out of scope when the response is returned by the controller. But the StreamedResponse has yet to return anything!

To keep the lock while the response is generated, it must be passed to the function executed by the StreamedResponse:

public function export(LockFactory $factory): Response
{
    // create a lock with a TTL of 60s
    $lock = $factory->createLock("test", 60);
    if (!$lock->acquire(false)) {
        return new Response("Too many downloads", Response::HTTP_TOO_MANY_REQUESTS);
    }

    $response = new StreamedResponse(function () use ($lock) {
        // now $lock is still alive when this function is executed
        $lockTime = time();
        while (have_some_data_to_output()) {
            if (time() - $lockTime > 50) {
                // refresh the lock well before it expires to be on safe side
                $lock->refresh();
                $lockTime = time();
            }
            output_data();
        }
        $lock->release();
    };

    $response->headers->set('Content-Type', 'text/csv');

    // lock would be released here if it wasn't passed to the StreamedResponse
    return $response;
}

The above code refreshes the lock every 50s to cut down on communication time with the storage engine (such as redis).

The lock remains locked for at most 60s should the php process suddenly die.

Upvotes: 0

Related Questions