Fixing deadlock and condition race errrors

taiga/base/api/mixins.py

@@ -54,7 +54,7 @@ from .settings import api_settings
 from .utils import get_object_or_404
 
 from .. import exceptions as exc
-
+from ..decorators import model_pk_lock
 
 def _get_validation_exclusions(obj, pk=None, slug_field=None, lookup_field=None):
     """
@@ -161,12 +161,15 @@ class UpdateModelMixin:
     """
 
     @tx.atomic
+    @model_pk_lock
     def update(self, request, *args, **kwargs):
         partial = kwargs.pop('partial', False)
         self.object = self.get_object_or_none()
-
         self.check_permissions(request, 'update', self.object)
 
+        if self.object is None:
+            raise Http404
+
         serializer = self.get_serializer(self.object, data=request.DATA,
                                          files=request.FILES, partial=partial)
 
@@ -227,6 +230,7 @@ class DestroyModelMixin:
     Destroy a model instance.
     """
     @tx.atomic
+    @model_pk_lock
     def destroy(self, request, *args, **kwargs):
         obj = self.get_object_or_none()
         self.check_permissions(request, 'destroy', obj)

taiga/base/decorators.py

@@ -15,8 +15,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-
-# Rest Framework 2.4 backport some decorators.
+from django_pglocks import advisory_lock
 
 def detail_route(methods=['get'], **kwargs):
     """
@@ -42,3 +41,21 @@ def list_route(methods=['get'], **kwargs):
         func.kwargs = kwargs
         return func
     return decorator
+
+
+def model_pk_lock(func):
+    """
+    This decorator is designed to be used in ModelViewsets methods to lock them based
+    on the model and the id of the selected object.
+    """
+    def decorator(self, *args, **kwargs):
+        from taiga.base.utils.db import get_typename_for_model_class
+        lookup_url_kwarg = self.lookup_url_kwarg or self.lookup_field
+        pk = self.kwargs.get(self.pk_url_kwarg, None)
+        tn = get_typename_for_model_class(self.get_queryset().model)
+        key = "{0}:{1}".format(tn, pk)
+
+        with advisory_lock(key) as acquired_key_lock:
+            return func(self, *args, **kwargs)
+
+    return decorator

taiga/base/utils/db.py

@@ -19,6 +19,8 @@ from django.contrib.contenttypes.models import ContentType
 from django.db import transaction
 from django.shortcuts import _get_queryset
 
+from django_pglocks import advisory_lock
+
 from . import functions
 
 import re
@@ -116,7 +118,6 @@ def update_in_bulk(instances, list_of_new_values, callback=None, precall=None):
         callback(instance)
 
 
-@transaction.atomic
 def update_in_bulk_with_ids(ids, list_of_new_values, model):
     """Update a table using a list of ids.
 
@@ -125,8 +126,11 @@ def update_in_bulk_with_ids(ids, list_of_new_values, model):
     to the instance in the same index position as the dict.
     :param model: Model of the ids.
     """
+    tn = get_typename_for_model_class(model)
     for id, new_values in zip(ids, list_of_new_values):
-        model.objects.filter(id=id).update(**new_values)
+        key = "{0}:{1}".format(tn, id)
+        with advisory_lock(key) as acquired_key_lock:
+            model.objects.filter(id=id).update(**new_values)
 
 
 def to_tsquery(term):

taiga/events/signal_handlers.py

@@ -55,5 +55,6 @@ def on_delete_any_model(sender, instance, **kwargs):
         return
 
     sesionid = mw.get_current_session_id()
-    events.emit_event_for_model(instance, sessionid=sesionid, type="delete")
-    
+
+    emit_event = lambda: events.emit_event_for_model(instance, sessionid=sesionid, type="delete")
+    connection.on_commit(emit_event)

taiga/projects/userstories/apps.py

@@ -23,6 +23,17 @@ from django.db.models import signals
 def connect_userstories_signals():
     from taiga.projects import signals as generic_handlers
     from . import signals as handlers
+
+    # When deleting user stories we must disable task signals while delating and
+    # enabling them in the end
+    signals.pre_delete.connect(handlers.disable_task_signals,
+                               sender=apps.get_model("userstories", "UserStory"),
+                               dispatch_uid='disable_task_signals')
+
+    signals.post_delete.connect(handlers.enable_tasks_signals,
+                           sender=apps.get_model("userstories", "UserStory"),
+                           dispatch_uid='enable_tasks_signals')
+
     # Cached prev object version
     signals.pre_save.connect(handlers.cached_prev_us,
                              sender=apps.get_model("userstories", "UserStory"),

taiga/projects/userstories/signals.py

@@ -18,6 +18,17 @@
 from contextlib import suppress
 from django.core.exceptions import ObjectDoesNotExist
 from taiga.projects.history.services import take_snapshot
+from taiga.projects.tasks.apps import connect_all_tasks_signals, disconnect_all_tasks_signals
+
+
+# Enable tasks signals
+def enable_tasks_signals(sender, instance, **kwargs):
+    connect_all_tasks_signals()
+
+
+# Disable tasks signals
+def disable_task_signals(sender, instance, **kwargs):
+    disconnect_all_tasks_signals()
 
 ####################################
 # Signals for cached prev US

tests/integration/resources_permissions/test_storage_resources.py

@@ -61,7 +61,7 @@ def test_storage_update(client, data):
     storage_data["key"] = "test"
     storage_data = json.dumps(storage_data)
     results = helper_test_http_method(client, 'put', url, storage_data, users)
-    assert results == [401, 200, 201]
+    assert results == [401, 200, 404]
 
 
 def test_storage_delete(client, data):
@@ -118,4 +118,4 @@ def test_storage_patch(client, data):
 
     patch_data = json.dumps({"value": {"test": "test-value"}})
     results = helper_test_http_method(client, 'patch', url, patch_data, users)
-    assert results == [401, 200, 201]
+    assert results == [401, 200, 404]

tests/integration/test_userstorage_api.py

@@ -144,10 +144,7 @@ def test_update_entries(client):
     assert response.status_code == 404
     response = client.json.put(reverse("user-storage-detail", args=[form["key"]]),
                                json.dumps(form))
-    assert response.status_code == 201
-    response = client.json.get(reverse("user-storage-detail", args=[form["key"]]))
-    assert response.status_code == 200
-    assert response.data["value"] == form["value"]
+    assert response.status_code == 404
 
 
 def test_delete_storage_entry(client):

tests/unit/test_utils.py

@@ -14,6 +14,7 @@
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
+import pytest
 
 from unittest import mock
 
@@ -23,6 +24,8 @@ import re
 from taiga.base.utils.urls import get_absolute_url, is_absolute_url, build_url
 from taiga.base.utils.db import save_in_bulk, update_in_bulk, update_in_bulk_with_ids, to_tsquery
 
+pytestmark = pytest.mark.django_db
+
 
 def test_is_absolute_url():
     assert is_absolute_url("http://domain/path")