green.thread: unlocked Lock().release() should raise exception, returned True

https://github.com/eventlet/eventlet/issues/697
https://github.com/eventlet/eventlet/pull/721

Co-authored-by: Sergey Shepelev <temotor@gmail.com>
Co-authored-by: Tim Burke <tim.burke@gmail.com>

5 files changed, 55 insertions, 28 deletions

diff --git a/eventlet/green/thread.py b/eventlet/green/thread.py
index 5800b30..e26f6b3 100644
--- a/eventlet/green/thread.py
+++ b/eventlet/green/thread.py
@@ -3,15 +3,16 @@ from six.moves import _thread as __thread
 import six
 from eventlet.support import greenlets as greenlet
 from eventlet import greenthread
-from eventlet.semaphore import Semaphore as LockType
+from eventlet.lock import Lock
 import sys
 
 
 __patched__ = ['get_ident', 'start_new_thread', 'start_new', 'allocate_lock',
                'allocate', 'exit', 'interrupt_main', 'stack_size', '_local',
-               'LockType', '_count']
+               'LockType', 'Lock', '_count']
 
 error = __thread.error
+LockType = Lock
 __threadcount = 0
 
 
diff --git a/eventlet/lock.py b/eventlet/lock.py
new file mode 100644
index 0000000..c665e19
--- /dev/null
+++ b/eventlet/lock.py
@@ -0,0 +1,28 @@
+from eventlet.semaphore import Semaphore
+
+
+class Lock(Semaphore):
+
+    """A lock.
+    This is API-compatible with :class:`threading.Lock`.
+
+    It is a context manager, and thus can be used in a with block::
+
+      lock = Lock()
+      with lock:
+        do_some_stuff()
+    """
+
+    def release(self, blocking=True):
+        """Modify behaviour vs :class:`Semaphore` to raise a RuntimeError
+        exception if the value is greater than zero. This corrects behaviour
+        to realign with :class:`threading.Lock`.
+        """
+        if self.counter > 0:
+            raise RuntimeError("release unlocked lock")
+
+        return super(Lock, self).release(blocking=blocking)
+
+    def _at_fork_reinit(self):
+        self.counter = 1
+        self._waiters.clear()
diff --git a/eventlet/semaphore.py b/eventlet/semaphore.py
index 5e2b5e3..18b5b05 100644
--- a/eventlet/semaphore.py
+++ b/eventlet/semaphore.py
@@ -39,7 +39,6 @@ class Semaphore(object):
         if value < 0:
             msg = 'Semaphore() expect value >= 0, actual: {0}'.format(repr(value))
             raise ValueError(msg)
-        self._original_value = value
         self.counter = value
         self._waiters = collections.deque()
 
@@ -52,10 +51,6 @@ class Semaphore(object):
         params = (self.__class__.__name__, self.counter, len(self._waiters))
         return '<%s c=%s _w[%s]>' % params
 
-    def _at_fork_reinit(self):
-        self.counter = self._original_value
-        self._waiters.clear()
-
     def locked(self):
         """Returns true if a call to acquire would block.
         """
diff --git a/tests/semaphore_test.py b/tests/semaphore_test.py
index cf6a29d..d6c11d1 100644
--- a/tests/semaphore_test.py
+++ b/tests/semaphore_test.py
@@ -42,27 +42,6 @@ class TestSemaphore(tests.LimitedTestCase):
         sem = eventlet.Semaphore()
         self.assertRaises(ValueError, sem.acquire, blocking=False, timeout=1)
 
-    def test_reinit(self):
-        # py39+ expects locks to have a _at_fork_reinit() method; since we
-        # patch in Semaphores in eventlet.green.thread, they need it, too
-        sem = eventlet.Semaphore()
-        sem.acquire()
-        sem._at_fork_reinit()
-        self.assertEqual(sem.acquire(blocking=False), True)
-        self.assertEqual(sem.acquire(blocking=False), False)
-
-        sem = eventlet.Semaphore(0)
-        sem.release()
-        sem._at_fork_reinit()
-        self.assertEqual(sem.acquire(blocking=False), False)
-
-        sem = eventlet.Semaphore(2)
-        sem.acquire()
-        sem._at_fork_reinit()
-        self.assertEqual(sem.acquire(blocking=False), True)
-        self.assertEqual(sem.acquire(blocking=False), True)
-        self.assertEqual(sem.acquire(blocking=False), False)
-
 
 def test_semaphore_contention():
     g_mutex = eventlet.Semaphore()
diff --git a/tests/thread_test.py b/tests/thread_test.py
index 44de95d..76e9b24 100644
--- a/tests/thread_test.py
+++ b/tests/thread_test.py
@@ -5,6 +5,7 @@ import eventlet
 from eventlet import corolocal
 from eventlet import event
 from eventlet import greenthread
+from eventlet import patcher
 from eventlet.green import thread
 import six
 
@@ -99,3 +100,26 @@ class Locals(LimitedTestCase):
         gc.collect()
         # at this point all our coros have terminated
         self.assertEqual(len(refs), 1)
+
+
+def test_compat_lock_release():
+    # https://github.com/eventlet/eventlet/issues/697
+    for mod in (patcher.original("threading"), thread):
+        try:
+            mod.Lock().release()
+        except RuntimeError as e:
+            # python3
+            assert "release unlocked lock" in str(e).lower(), str((mod, e))
+        except thread.error as e:
+            # python2.7
+            assert "release unlocked lock" in str(e).lower(), str((mod, e))
+
+
+def test_reinit():
+    # py39+ expects locks to have a _at_fork_reinit() method
+    # https://github.com/eventlet/eventlet/pull/721#pullrequestreview-769377850
+    lk = thread.Lock()
+    lk.acquire()
+    lk._at_fork_reinit()
+    assert lk.acquire(blocking=False)
+    assert not lk.acquire(blocking=False)