From ad00ce67c3a158c722a5e96f48d8ac05fbac785c Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Wed, 18 Dec 2019 15:39:13 +0100 Subject: [PATCH 1/2] vendor RWLock --- src/ecdsa/_rwlock.py | 85 ++++++++++++++++++ src/ecdsa/test_rw_lock.py | 175 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 260 insertions(+) create mode 100644 src/ecdsa/_rwlock.py create mode 100644 src/ecdsa/test_rw_lock.py diff --git a/src/ecdsa/_rwlock.py b/src/ecdsa/_rwlock.py new file mode 100644 index 00000000..e4ef78dc --- /dev/null +++ b/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() diff --git a/src/ecdsa/test_rw_lock.py b/src/ecdsa/test_rw_lock.py new file mode 100644 index 00000000..de11d156 --- /dev/null +++ b/src/ecdsa/test_rw_lock.py @@ -0,0 +1,175 @@ +# Copyright Mateusz Kobos, (c) 2011 +# https://code.activestate.com/recipes/577803-reader-writer-lock-with-priority-for-writers/ +# released under the MIT licence + +import unittest +import threading +import time +import copy +from ._rwlock import RWLock + + +class Writer(threading.Thread): + def __init__(self, buffer_, rw_lock, init_sleep_time, sleep_time, to_write): + """ + @param buffer_: common buffer_ shared by the readers and writers + @type buffer_: list + @type rw_lock: L{RWLock} + @param init_sleep_time: sleep time before doing any action + @type init_sleep_time: C{float} + @param sleep_time: sleep time while in critical section + @type sleep_time: C{float} + @param to_write: data that will be appended to the buffer + """ + threading.Thread.__init__(self) + self.__buffer = buffer_ + self.__rw_lock = rw_lock + self.__init_sleep_time = init_sleep_time + self.__sleep_time = sleep_time + self.__to_write = to_write + self.entry_time = None + """Time of entry to the critical section""" + self.exit_time = None + """Time of exit from the critical section""" + + def run(self): + time.sleep(self.__init_sleep_time) + self.__rw_lock.writer_acquire() + self.entry_time = time.time() + time.sleep(self.__sleep_time) + self.__buffer.append(self.__to_write) + self.exit_time = time.time() + self.__rw_lock.writer_release() + + +class Reader(threading.Thread): + def __init__(self, buffer_, rw_lock, init_sleep_time, sleep_time): + """ + @param buffer_: common buffer shared by the readers and writers + @type buffer_: list + @type rw_lock: L{RWLock} + @param init_sleep_time: sleep time before doing any action + @type init_sleep_time: C{float} + @param sleep_time: sleep time while in critical section + @type sleep_time: C{float} + """ + threading.Thread.__init__(self) + self.__buffer = buffer_ + self.__rw_lock = rw_lock + self.__init_sleep_time = init_sleep_time + self.__sleep_time = sleep_time + self.buffer_read = None + """a copy of a the buffer read while in critical section""" + self.entry_time = None + """Time of entry to the critical section""" + self.exit_time = None + """Time of exit from the critical section""" + + def run(self): + time.sleep(self.__init_sleep_time) + self.__rw_lock.reader_acquire() + self.entry_time = time.time() + time.sleep(self.__sleep_time) + self.buffer_read = copy.deepcopy(self.__buffer) + self.exit_time = time.time() + self.__rw_lock.reader_release() + + +class RWLockTestCase(unittest.TestCase): + def test_readers_nonexclusive_access(self): + (buffer_, rw_lock, threads) = self.__init_variables() + + threads.append(Reader(buffer_, rw_lock, 0, 0)) + threads.append(Writer(buffer_, rw_lock, 0.2, 0.4, 1)) + threads.append(Reader(buffer_, rw_lock, 0.3, 0.3)) + threads.append(Reader(buffer_, rw_lock, 0.5, 0)) + + self.__start_and_join_threads(threads) + + ## The third reader should enter after the second one but it should + ## exit before the second one exits + ## (i.e. the readers should be in the critical section + ## at the same time) + + self.assertEqual([], threads[0].buffer_read) + self.assertEqual([1], threads[2].buffer_read) + self.assertEqual([1], threads[3].buffer_read) + self.assert_(threads[1].exit_time <= threads[2].entry_time) + self.assert_(threads[2].entry_time <= threads[3].entry_time) + self.assert_(threads[3].exit_time < threads[2].exit_time) + + def test_writers_exclusive_access(self): + (buffer_, rw_lock, threads) = self.__init_variables() + + threads.append(Writer(buffer_, rw_lock, 0, 0.4, 1)) + threads.append(Writer(buffer_, rw_lock, 0.1, 0, 2)) + threads.append(Reader(buffer_, rw_lock, 0.2, 0)) + + self.__start_and_join_threads(threads) + + ## The second writer should wait for the first one to exit + + self.assertEqual([1, 2], threads[2].buffer_read) + self.assert_(threads[0].exit_time <= threads[1].entry_time) + self.assert_(threads[1].exit_time <= threads[2].exit_time) + + def test_writer_priority(self): + (buffer_, rw_lock, threads) = self.__init_variables() + + threads.append(Writer(buffer_, rw_lock, 0, 0, 1)) + threads.append(Reader(buffer_, rw_lock, 0.1, 0.4)) + threads.append(Writer(buffer_, rw_lock, 0.2, 0, 2)) + threads.append(Reader(buffer_, rw_lock, 0.3, 0)) + threads.append(Reader(buffer_, rw_lock, 0.3, 0)) + + self.__start_and_join_threads(threads) + + ## The second writer should go before the second and the third reader + + self.assertEqual([1], threads[1].buffer_read) + self.assertEqual([1, 2], threads[3].buffer_read) + self.assertEqual([1, 2], threads[4].buffer_read) + self.assert_(threads[0].exit_time < threads[1].entry_time) + self.assert_(threads[1].exit_time <= threads[2].entry_time) + self.assert_(threads[2].exit_time <= threads[3].entry_time) + self.assert_(threads[2].exit_time <= threads[4].entry_time) + + def test_many_writers_priority(self): + (buffer_, rw_lock, threads) = self.__init_variables() + + threads.append(Writer(buffer_, rw_lock, 0, 0, 1)) + threads.append(Reader(buffer_, rw_lock, 0.1, 0.6)) + threads.append(Writer(buffer_, rw_lock, 0.2, 0.1, 2)) + threads.append(Reader(buffer_, rw_lock, 0.3, 0)) + threads.append(Reader(buffer_, rw_lock, 0.4, 0)) + threads.append(Writer(buffer_, rw_lock, 0.5, 0.1, 3)) + + self.__start_and_join_threads(threads) + + ## The two last writers should go first -- after the first reader and + ## before the second and the third reader + + self.assertEqual([1], threads[1].buffer_read) + self.assertEqual([1, 2, 3], threads[3].buffer_read) + self.assertEqual([1, 2, 3], threads[4].buffer_read) + self.assert_(threads[0].exit_time < threads[1].entry_time) + self.assert_(threads[1].exit_time <= threads[2].entry_time) + self.assert_(threads[1].exit_time <= threads[5].entry_time) + self.assert_(threads[2].exit_time <= threads[3].entry_time) + self.assert_(threads[2].exit_time <= threads[4].entry_time) + self.assert_(threads[5].exit_time <= threads[3].entry_time) + self.assert_(threads[5].exit_time <= threads[4].entry_time) + + @staticmethod + def __init_variables(): + buffer_ = [] + rw_lock = RWLock() + threads = [] + return (buffer_, rw_lock, threads) + + @staticmethod + def __start_and_join_threads(threads): + for t in threads: + t.start() + for t in threads: + t.join() From 1d3b3c62e26331cfe5cdc29ca356a64c6e5f5de0 Mon Sep 17 00:00:00 2001 From: Hubert Kario Date: Mon, 16 Dec 2019 17:09:04 +0100 Subject: [PATCH 2/2] make scale() thread-safe --- src/ecdsa/ellipticcurve.py | 107 ++++++++++++++++++++++++++++--------- 1 file changed, 82 insertions(+), 25 deletions(-) diff --git a/src/ecdsa/ellipticcurve.py b/src/ecdsa/ellipticcurve.py index 7fb74d9a..3420454d 100644 --- a/src/ecdsa/ellipticcurve.py +++ b/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,11 +245,17 @@ 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): """ @@ -241,12 +263,28 @@ def scale(self): 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): @@ -254,6 +292,7 @@ def to_affine(self): 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):