[FIXED] How to prevent race condition while working with sessions?

Issue

I’m working on an e-commerce website with a cart and products. A product’s primary key is added to the user’s session data in a dictionary 'cart_content'. This dictionary hold product’s primary key as key and amount of the given product as value.

I successfully reproduced a race condition bug by adding simultaneously a product twice with an amount big enough so only one of the two additions sold out the product (e.g. 3 products left and adding 3 twice). The two requests increment twice the cart but there is not enough stock (in our example 3 left but 6 in cart).

How can I prevent race condition to happen like the example given above or in a multi-user case ? Does there is for example some sort of lock system that prevent add_to_cart() from being executed asynchronously ?

Edit: I switched from SQLite to PostgreSQL because I wanted to test select_for_update().

core/models.py :

class Product(models.Model):
    …
    number_in_stock = models.IntegerField(default=0)
    
    @property
    def number_on_hold(self):
        result = 0
        for s in Session.objects.all():
            amount = s.get_decoded().get('cart_content', {}).get(str(self.pk))
            if amount is not None:
                result += int(amount)
        return result
    …

cart/views.py :

def add_to_cart(request):
    if (request.method == "POST"):
        pk = request.POST.get('pk', None)
        amount = int(request.POST.get('amount', 0))
        if pk and amount:
            p = Product.objects.get(pk=pk)
            if amount > p.number_in_stock - p.number_on_hold:
                return HttpResponse('1')
            if not request.session.get('cart_content', None):
                request.session['cart_content'] = {}
            if request.session['cart_content'].get(pk, None):
                request.session['cart_content'][pk] += amount
            else:
                request.session['cart_content'][pk] = amount
            request.session.modified = True
            return HttpResponse('0')
    return HttpResponse('', status=404)

cart/urls.py:

urlpatterns = [
    …
    path("add-to-cart", views.add_to_cart, name="cart-add-to-cart"),
    …
]

cart/templates/cart/html/atoms/add_to_cart.html :

<div class="form-element add-to-cart-amount">
    <label for="addToCartAmount{{ product_pk }}"> {% translate "Amount" %} : </label>
    <input type="number" id="addToCartAmount{{ product_pk }}" />
</div>
<div class="form-element add-to-cart">
    <button class="btn btn-primary button-add-to-cart" data-product-pk="{{ product_pk }}" data-href="{% url 'cart-add-to-cart' %}"><span> {% translate "Add to cart" %} </span></button>
</div>

cart/static/cart/js/main.js:

$(".button-add-to-cart").click(function(event) {
    event.preventDefault();
    let product_pk = $(this).data("productPk");
    let amount = $(this).parent().parent().find("#addToCartAmount" + product_pk).val();
    $.ajax({
        url: $(this).data("href"),
        method: 'POST',
        data: {
            pk: product_pk,
            amount: amount
        },
        success: function(result) {
            switch (result) {
                case '1':
                    alert(gettext('Amount exceeded'));
                    break;
                case '0':
                    alert(interpolate(gettext('Successfully added %s items to the cart.'), [amount]))
                    break;
                default:
                    alert(gettext('Unknown error'));
            }
        }
    });
});

The Javascript used to reproduce the race condition. Of course it doesn’t always work the first time. Just repeat two or three times until you get the behaviour I mentioned :

async function test() {
    document.getElementsByClassName('button-add-to-cart')[0].click();
}

test(); test();

Solution

Edit with full solution - Short answer

Use @transaction.atomic with select_for_update() and manually save the session with save(). You have to select all sessions and evaluate with list() so it will lock every sessions. This is because of a multi-user scenario. This isn’t working for SQLite :

@transaction.atomic
def _add_to_cart_atomic(request, product_id, amount):
    p = Product.objects.get(pk=product_id)
    list(Session.objects.select_for_update())  # Force evaluation to lock all the sessions
    if amount > p.number_in_stock - p.number_on_hold:
        return HttpResponse('1')
    if not request.session.get('cart_content', None):
        request.session['cart_content'] = {}
    if request.session['cart_content'].get(product_id, None):
        request.session['cart_content'][product_id] += amount
    else:
        request.session['cart_content'][product_id] = amount
    request.session.save()
    return HttpResponse('0')

def add_to_cart(request):
    if (request.method == "POST"):
        pk = request.POST.get('pk', None)
        amount = int(request.POST.get('amount', 0))
        if pk and amount:
            return _add_to_cart_atomic(request, pk, amount)
    return HttpResponse('', status=404)

---

Long answer

Using @transaction.atomic alone

Using @transaction.atomic decorator seems to resolve half of the issue, even if I can’t really prove this do works 100%. This works for both SQLite and PostgreSQL.

From Wikipedia:

An atomic transaction is an indivisible and irreducible series of database operations such that either all occur, or none occur. A guarantee of atomicity prevents partial database updates from occurring, because they can cause greater problems than rejecting the whole series outright.

@transaction.atomic
def add_to_cart_atomic(request, pk, amount):
    p = Product.objects.get(pk=pk)
    if amount > p.number_in_stock - p.number_on_hold:
        return HttpResponse('1')
    if not request.session.get('cart_content', None):
        request.session['cart_content'] = {}
    if request.session['cart_content'].get(pk, None):
        request.session['cart_content'][pk] += amount
    else:
        request.session['cart_content'][pk] = amount
    request.session.modified = True
    return HttpResponse('0')

def add_to_cart(request):
    if (request.method == "POST"):
        pk = request.POST.get('pk', None)
        amount = int(request.POST.get('amount', 0))
        if pk and amount:
            return add_to_cart_atomic(request, pk, amount)
    return HttpResponse('', status=404)

But it raises another problem. When I redo my double click javascript test, sometimes, I get the "Success" alert for both of the requests, even if the server doesn’t really add twice to the cart. I think this is because the SQL query is dropped without any error raised. And sometimes, it works as expected, the first request gets a success and the second a "Amount exceeded".

Using a lock on the current session

This is almost Ilya Khrustalev’s answer. If we add a select_for_update() on the current session. This does not work for SQLite:

@transaction.atomic
def _add_to_cart_atomic(request, product_id, amount):
    p = Product.objects.get(pk=product_id)
    Session.objects.select_for_update().get(session_key = request.session.session_key)
    …

Then this will only lock for the current session and so our little Javascript test will pass. But what about a multi-user scenario like this ? :

async def add_one_to_cart(session_key):
    async with aiohttp.ClientSession(cookies={'sessionid': session_key}) as session:
        async with session.post('http://localhost:8000/cart/add-to-cart', data={'pk': 1, 'amount': 1}, cookies={'sessionid': session_key}) as response:
            return await response.text()

async def main():
    result = await asyncio.gather(
        add_one_to_cart('session_key_A'),
        add_one_to_cart('session_key_B'),
        add_one_to_cart('session_key_C')
    )
    print(result)

asyncio.run(main())

Result:

['0', '0', '0']

Meaning every request from the three different session pass even if there is only one product left! So we have a number_on_hold that exceed the number_on_stock of 2 !

Using a lock on all sessions

This is my "Short answer".

Answered By - Zatigem