Fix UUID race conditions

python-packages/securesync/devices/models.py

@@ -10,6 +10,7 @@
 from django.core.exceptions import ValidationError, ObjectDoesNotExist
 from django.db import models, transaction
 from django.db.models import Q
+from django.db.models.expressions import F
 from django.utils.text import compress_string
 from django.utils.translation import ugettext_lazy as _
 
@@ -214,10 +215,7 @@ def _hashable_representation(self):
         return super(Device, self)._hashable_representation(fields=fields)
 
     def get_metadata(self):
-        try:
-            return self.devicemetadata
-        except DeviceMetadata.DoesNotExist:
-            return DeviceMetadata(device=self)
+        return DeviceMetadata.objects.get_or_create(device=self)[0]
 
     def get_counter_position(self):
         """
@@ -243,15 +241,16 @@ def set_counter_position(self, counter_position, soft_set=False):
         metadata.counter_position = counter_position
         metadata.save()
 
-
     @transaction.commit_on_success
     def increment_counter_position(self):
+        """Increment and return the counter position for this device.
+        TODO-BLOCKER(jamalex): While from testing, this new version seems much less prone to race conditions,
+        it still seems like a possibility. Further testing would be good.
+        """
         metadata = self.get_metadata()
-        if not metadata.device.id:
-            return 0
-        metadata.counter_position += 1
+        metadata.counter_position = F("counter_position") + 1
         metadata.save()
-        return metadata.counter_position
+        return self.get_metadata().counter_position
 
     def full_clean(self, *args, **kwargs):
         # TODO(jamalex): we skip out here, because otherwise self-signed devices will fail

python-packages/securesync/engine/models.py

@@ -388,16 +388,24 @@ def save(self, imported=False, increment_counters=True, sign=True, *args, **kwar
     def set_id(self):
         self.id = self.id or self.get_uuid()
 
+    # def get_uuid(self):
+    #     """
+    #     By default, all objects get an ID from the
+    #     device and the counter position at which it was created.
+    #     """
+    #     assert self.counter is not None, "counter required for get_uuid"
+
+    #     own_device = _get_own_device()
+    #     namespace = own_device.id and uuid.UUID(own_device.id) or settings.ROOT_UUID_NAMESPACE
+    #     return uuid.uuid5(namespace, str(self.counter)).hex
+
     def get_uuid(self):
         """
-        By default, all objects get an ID from the
-        device and the counter position at which it was created.
+        Replacement default UUID generator that is completely random, rather than being based
+        on the signing device id and counter, until we can make the increment_counter_position
+        function properly atomic, to avoid a race condition leading to duplicate counters/IDs.
         """
-        assert self.counter is not None, "counter required for get_uuid"
-
-        own_device = _get_own_device()
-        namespace = own_device.id and uuid.UUID(own_device.id) or settings.ROOT_UUID_NAMESPACE
-        return uuid.uuid5(namespace, str(self.counter)).hex
+        return uuid.uuid4().hex
 
     def get_existing_instance(self):
         uuid = self.id or self.get_uuid()
@@ -470,17 +478,7 @@ def save(self, increment_counters=settings.CENTRAL_SERVER, *args, **kwargs):
         super(DeferredCountSyncedModel, self).save(*args, increment_counters=increment_counters, **kwargs)
 
     def set_id(self):
-        if self.id:
-            pass
-        elif self.counter:
-            self.id = self.get_uuid()
-        else:
-            # UUID depends on counter position, so we *have* to get a counter
-            #   position to set an id
-            own_device = _get_own_device()
-            self.counter = own_device.increment_counter_position()
-            self.id = self.get_uuid()
-            self.counter = None
+        self.id = self.id or self.get_uuid()
 
     class Meta:  # needed to clear out the app_name property from SyncedClass.Meta
         app_label = "securesync"