Merge pull request #177 from tomato42/thread-safe-scale

make scale() thread-safe

src/ecdsa/_rwlock.py

@@ -0,0 +1,85 @@
+# Copyright Mateusz Kobos, (c) 2011
+# https://code.activestate.com/recipes/577803-reader-writer-lock-with-priority-for-writers/
+# released under the MIT licence
+
+import threading
+
+
+__author__ = "Mateusz Kobos"
+
+
+class RWLock:
+    """
+    Read-Write locking primitive
+
+    Synchronization object used in a solution of so-called second
+    readers-writers problem. In this problem, many readers can simultaneously
+    access a share, and a writer has an exclusive access to this share.
+    Additionally, the following constraints should be met:
+    1) no reader should be kept waiting if the share is currently opened for
+       reading unless a writer is also waiting for the share,
+    2) no writer should be kept waiting for the share longer than absolutely
+       necessary.
+
+    The implementation is based on [1, secs. 4.2.2, 4.2.6, 4.2.7]
+    with a modification -- adding an additional lock (C{self.__readers_queue})
+    -- in accordance with [2].
+
+    Sources:
+    [1] A.B. Downey: "The little book of semaphores", Version 2.1.5, 2008
+    [2] P.J. Courtois, F. Heymans, D.L. Parnas:
+        "Concurrent Control with 'Readers' and 'Writers'",
+        Communications of the ACM, 1971 (via [3])
+    [3] http://en.wikipedia.org/wiki/Readers-writers_problem
+    """
+
+    def __init__(self):
+        """
+        A lock giving an even higher priority to the writer in certain
+        cases (see [2] for a discussion).
+        """
+        self.__read_switch = _LightSwitch()
+        self.__write_switch = _LightSwitch()
+        self.__no_readers = threading.Lock()
+        self.__no_writers = threading.Lock()
+        self.__readers_queue = threading.Lock()
+
+    def reader_acquire(self):
+        self.__readers_queue.acquire()
+        self.__no_readers.acquire()
+        self.__read_switch.acquire(self.__no_writers)
+        self.__no_readers.release()
+        self.__readers_queue.release()
+
+    def reader_release(self):
+        self.__read_switch.release(self.__no_writers)
+
+    def writer_acquire(self):
+        self.__write_switch.acquire(self.__no_readers)
+        self.__no_writers.acquire()
+
+    def writer_release(self):
+        self.__no_writers.release()
+        self.__write_switch.release(self.__no_readers)
+
+
+class _LightSwitch:
+    """An auxiliary "light switch"-like object. The first thread turns on the
+    "switch", the last one turns it off (see [1, sec. 4.2.2] for details)."""
+    def __init__(self):
+        self.__counter = 0
+        self.__mutex = threading.Lock()
+
+    def acquire(self, lock):
+        self.__mutex.acquire()
+        self.__counter += 1
+        if self.__counter == 1:
+            lock.acquire()
+        self.__mutex.release()
+
+    def release(self, lock):
+        self.__mutex.acquire()
+        self.__counter -= 1
+        if self.__counter == 0:
+            lock.release()
+        self.__mutex.release()

src/ecdsa/ellipticcurve.py

@@ -48,6 +48,7 @@
 
 from six import python_2_unicode_compatible
 from . import numbertheory
+from ._rwlock import RWLock
 
 
 @python_2_unicode_compatible
@@ -145,6 +146,9 @@ def __init__(self, curve, x, y, z, order=None, generator=False):
         cause to precompute multiplication table for it
       """
       self.__curve = curve
+      # since it's generally better (faster) to use scaled points vs unscaled
+      # ones, use writer-biased RWLock for locking:
+      self._scale_lock = RWLock()
       if GMPY:
           self.__x = mpz(x)
           self.__y = mpz(y)
@@ -171,19 +175,25 @@ def __init__(self, curve, x, y, z, order=None, generator=False):
 
   def __eq__(self, other):
       """Compare two points with each-other."""
-      if (not self.__y or not self.__z) and other is INFINITY:
-          return True
-      if self.__y and self.__z and other is INFINITY:
-          return False
+      try:
+          self._scale_lock.reader_acquire()
+          if other is INFINITY:
+              return not self.__y or not self.__z
+          x1, y1, z1 = self.__x, self.__y, self.__z
+      finally:
+          self._scale_lock.reader_release()
       if isinstance(other, Point):
           x2, y2, z2 = other.x(), other.y(), 1
       elif isinstance(other, PointJacobi):
-          x2, y2, z2 = other.__x, other.__y, other.__z
+          try:
+              other._scale_lock.reader_acquire()
+              x2, y2, z2 = other.__x, other.__y, other.__z
+          finally:
+              other._scale_lock.reader_release()
       else:
           return NotImplemented
       if self.__curve != other.curve():
           return False
-      x1, y1, z1 = self.__x, self.__y, self.__z
       p = self.__curve.p()
 
       zz1 = z1 * z1 % p
@@ -214,11 +224,17 @@ def x(self):
       call x() and y() on the returned instance. Or call `scale()`
       and then x() and y() on the returned instance.
       """
-      if self.__z == 1:
-          return self.__x
+      try:
+          self._scale_lock.reader_acquire()
+          if self.__z == 1:
+              return self.__x
+          x = self.__x
+          z = self.__z
+      finally:
+          self._scale_lock.reader_release()
       p = self.__curve.p()
-      z = numbertheory.inverse_mod(self.__z, p)
-      return self.__x * z**2 % p
+      z = numbertheory.inverse_mod(z, p)
+      return x * z**2 % p
 
   def y(self):
       """
@@ -229,31 +245,54 @@ def y(self):
       call x() and y() on the returned instance. Or call `scale()`
       and then x() and y() on the returned instance.
       """
-      if self.__z == 1:
-          return self.__y
+      try:
+          self._scale_lock.reader_acquire()
+          if self.__z == 1:
+              return self.__y
+          y = self.__y
+          z = self.__z
+      finally:
+          self._scale_lock.reader_release()
       p = self.__curve.p()
-      z = numbertheory.inverse_mod(self.__z, p)
-      return self.__y * z**3 % p
+      z = numbertheory.inverse_mod(z, p)
+      return y * z**3 % p
 
   def scale(self):
       """
       Return point scaled so that z == 1.
 
       Modifies point in place, returns self.
       """
-      p = self.__curve.p()
-      z_inv = numbertheory.inverse_mod(self.__z, p)
-      zz_inv = z_inv * z_inv % p
-      self.__x = self.__x * zz_inv % p
-      self.__y = self.__y * zz_inv * z_inv % p
-      self.__z = 1
+      try:
+          self._scale_lock.reader_acquire()
+          if self.__z == 1:
+              return self
+      finally:
+          self._scale_lock.reader_release()
+
+      try:
+          self._scale_lock.writer_acquire()
+          # scaling already scaled point is safe (as inverse of 1 is 1) and
+          # quick so we don't need to optimise for the unlikely event when
+          # two threads hit the lock at the same time
+          p = self.__curve.p()
+          z_inv = numbertheory.inverse_mod(self.__z, p)
+          zz_inv = z_inv * z_inv % p
+          self.__x = self.__x * zz_inv % p
+          self.__y = self.__y * zz_inv * z_inv % p
+          # we are setting the z last so that the check above will return true
+          # only after all values were already updated
+          self.__z = 1
+      finally:
+          self._scale_lock.writer_release()
       return self
 
   def to_affine(self):
       """Return point in affine form."""
       if not self.__y or not self.__z:
           return INFINITY
       self.scale()
+      # after point is scaled, it's immutable, so no need to perform locking
       return Point(self.__curve, self.__x,
                    self.__y, self.__order)
 
@@ -323,7 +362,11 @@ def double(self):
 
       p, a = self.__curve.p(), self.__curve.a()
 
-      X1, Y1, Z1 = self.__x, self.__y, self.__z
+      try:
+          self._scale_lock.reader_acquire()
+          X1, Y1, Z1 = self.__x, self.__y, self.__z
+      finally:
+          self._scale_lock.reader_release()
 
       X3, Y3, Z3 = self._double(X1, Y1, Z1, p, a)
 
@@ -437,8 +480,16 @@ def __add__(self, other):
           raise ValueError("The other point is on different curve")
 
       p = self.__curve.p()
-      X1, Y1, Z1 = self.__x, self.__y, self.__z
-      X2, Y2, Z2 = other.__x, other.__y, other.__z
+      try:
+          self._scale_lock.reader_acquire()
+          X1, Y1, Z1 = self.__x, self.__y, self.__z
+      finally:
+          self._scale_lock.reader_release()
+      try:
+          other._scale_lock.reader_acquire()
+          X2, Y2, Z2 = other.__x, other.__y, other.__z
+      finally:
+          other._scale_lock.reader_release()
       X3, Y3, Z3 = self._add(X1, Y1, Z1, X2, Y2, Z2, p)
 
       if not Y3 or not Z3:
@@ -497,6 +548,7 @@ def __mul__(self, other):
           return self._mul_precompute(other)
 
       self = self.scale()
+      # once scaled, point is immutable, not need to lock
       X2, Y2 = self.__x, self.__y
       X3, Y3, Z3 = 0, 0, 1
       p, a = self.__curve.p(), self.__curve.a()
@@ -550,6 +602,7 @@ def mul_add(self, self_mul, other, other_mul):
       X3, Y3, Z3 = 0, 0, 1
       p, a = self.__curve.p(), self.__curve.a()
       self = self.scale()
+      # after scaling, point is immutable, no need for locking
       X1, Y1 = self.__x, self.__y
       other = other.scale()
       X2, Y2 = other.__x, other.__y
@@ -575,8 +628,12 @@ def mul_add(self, self_mul, other, other_mul):
 
   def __neg__(self):
       """Return negated point."""
-      return PointJacobi(self.__curve, self.__x, -self.__y, self.__z,
-                         self.__order)
+      try:
+          self._scale_lock.reader_acquire()
+          return PointJacobi(self.__curve, self.__x, -self.__y, self.__z,
+                             self.__order)
+      finally:
+          self._scale_lock.reader_release()
 
 
 class Point(object):