Fix #704 by going back to pure-python version of Semaphore on PyPy.

The main stated reason for untraceable locks was a bug in Python 2.5,
fixed in all recent versions, so that shouldn't be a concern.

The semaphore takes some careful lock management to come close to
duplicating the thread-safety inherent in Cython's compiled methods.

'pypy -m timeit -s "import g.l.Semaphore; s=Semaphore()"' 's.acquire();s.releas()':

With Cython compiled semaphore: 5usec
With pure-python sempahore: 0.004usec
With pure-python-but-locked semaphore: 0.178usec
Cython on CPython: 0.141usec

Total test runtime is unchanged.

Makefile

@@ -23,15 +23,16 @@ gevent/gevent.ares.c: gevent/ares.pyx gevent/*.pxd
 	$(CYTHON) -o gevent.ares.c gevent/ares.pyx
 	mv gevent.ares.* gevent/
 
-gevent/gevent._semaphore.c: gevent/_semaphore.pyx gevent/_semaphore.pxd
-# For PyPy, we need to have _semaphore named as a .pyx file so it doesn't
-# get loaded in preference to the .so. But we want to keep the definitions
+gevent/gevent._semaphore.c: gevent/_semaphore.py gevent/_semaphore.pxd
+# On PyPy, if we wanted to use Cython to compile _semaphore.py, we'd
+# need to have _semaphore named as a .pyx file so it doesn't get
+# loaded in preference to the .so. (We want to keep the definitions
 # separate in a .pxd file for ease of reading, and that only works
-# with .py files.
-	cp gevent/_semaphore.pyx gevent/_semaphore.py
+# with .py files, so we'd have to copy them back and forth.)
+#	cp gevent/_semaphore.pyx gevent/_semaphore.py
 	$(CYTHON) -o gevent._semaphore.c gevent/_semaphore.py
 	mv gevent._semaphore.* gevent/
-	rm gevent/_semaphore.py
+#	rm gevent/_semaphore.py
 
 clean:
 	rm -f corecext.pyx gevent/corecext.pyx

appveyor/make.cmd

@@ -5,7 +5,5 @@ IF "%PYTHON_EXE%" == "python" (
 )
 cython -o gevent.ares.c gevent/ares.pyx
 move gevent.ares.* gevent
-move gevent\\_semaphore.pyx gevent\\_semaphore.py
 cython -o gevent._semaphore.c gevent/_semaphore.py
 move gevent._semaphore.* gevent
-del gevent\\_semaphore.py

gevent/lock.py

 # Copyright (c) 2009-2012 Denis Bilenko. See LICENSE for details.
 """Locking primitives"""
+from __future__ import absolute_import
 
-from gevent.hub import getcurrent
+from gevent.hub import getcurrent, PYPY
 from gevent._semaphore import Semaphore, BoundedSemaphore
 
 
 __all__ = ['Semaphore', 'DummySemaphore', 'BoundedSemaphore', 'RLock']
 
+# On PyPy, we don't compile the Semaphore class with Cython. Under
+# Cython, each individual method holds the GIL for its entire
+# duration, ensuring that no other thread can interrupt us in an
+# unsafe state (only when we _do_wait do we call back into Python and
+# allow switching threads). Simulate that here through the use of a manual
+# lock. (In theory we could use a lock per Semaphore and thus potentially
+# scale better than the GIL, but it probably doesn't matter.)
+if PYPY:
+    # TODO: Need to use monkey.get_original?
+    from thread import allocate_lock as _allocate_lock
+    _sem_lock = _allocate_lock()
+
+    # acquire, wait, and release all acquire the lock on entry and release it
+    # on exit. acquire and wait can call _do_wait, which must release it on entry
+    # and re-acquire it for them on exit.
+    class _locked(object):
+        def __enter__(self):
+            _sem_lock.acquire()
+
+        def __exit__(self, t, v, tb):
+            _sem_lock.release()
+
+    class _unlocked(object):
+
+        def __enter__(self):
+            _sem_lock.release()
+
+        def __exit__(self, t, v, tb):
+            _sem_lock.acquire()
+
+    def _decorate(func, cm):
+        # functools.wrap?
+        def wrapped(*args, **kwargs):
+            with cm:
+                return func(*args, **kwargs)
+        return wrapped
+
+    Semaphore._py3k_acquire = Semaphore.acquire = _decorate(Semaphore.acquire, _locked())
+    Semaphore.release = _decorate(Semaphore.release, _locked())
+    Semaphore.wait = _decorate(Semaphore.wait, _locked())
+    Semaphore._do_wait = _decorate(Semaphore._do_wait, _unlocked())
+
 
 class DummySemaphore(object):
     """

greentest/test__semaphore.py

@@ -32,6 +32,21 @@ class TestSemaphore(greentest.TestCase):
         r = weakref.ref(s)
         self.assertEqual(s, r())
 
+    def test_semaphore_in_class_with_del(self):
+        # Issue #704. This used to crash the process
+        # under PyPy through at least 4.0.1
+        class X(object):
+            def __init__(self):
+                self.s = Semaphore()
+
+            def __del__(self):
+                self.s.acquire()
+
+        X()
+        import gc
+        gc.collect()
+        gc.collect()
+
 
 class TestLock(greentest.TestCase):
 

greentest/test__threading_vs_settrace.py

@@ -2,7 +2,7 @@ from __future__ import print_function
 import sys
 import subprocess
 import unittest
-import gevent.thread
+from gevent.thread import allocate_lock
 
 script = """
 from gevent import monkey
@@ -52,19 +52,25 @@ class TestTrace(unittest.TestCase):
             old = sys.gettrace()
         else:
             old = None
+        PYPY = hasattr(sys, 'pypy_version_info')
         lst = []
         try:
             def trace(frame, ev, arg):
                 lst.append((frame.f_code.co_filename, frame.f_lineno, ev))
+                if not PYPY:
                     print("TRACE: %s:%s %s" % lst[-1])
                 return trace
 
-            with gevent.thread.allocate_lock():
+            with allocate_lock():
                 sys.settrace(trace)
         finally:
             sys.settrace(old)
 
-        self.failUnless(lst == [], "trace not empty")
+        if not PYPY:
+            self.assertEqual(lst, [], "trace not empty")
+        else:
+            # Have an assert so that we know if we miscompile
+            self.assertTrue(len(lst) > 0, "should not compile on pypy")
 
     def run_script(self, more_args=()):
         args = [sys.executable, "-c", script]

setup.py

@@ -417,11 +417,15 @@ elif PYPY:
         # and having only the bare minimum be in cython might help reduce that penalty.
         # NOTE: You must use version 0.23.4 or later to avoid a memory leak.
         # https://mail.python.org/pipermail/cython-devel/2015-October/004571.html
-        Extension(name="gevent._semaphore",
-                  sources=["gevent/gevent._semaphore.c"]),
+        # However, that's all for naught on up to and including PyPy 4.0.1 which
+        # have some serious crashing bugs with GC interacting with cython,
+        # so this is disabled (would need to add gevent/gevent._semaphore.c back to
+        # the run_make line)
+        #Extension(name="gevent._semaphore",
+        #          sources=["gevent/gevent._semaphore.c"]),
     ]
     include_package_data = True
-    run_make = 'gevent/gevent._semaphore.c gevent/gevent.ares.c'
+    run_make = 'gevent/gevent.ares.c'
 else:
     ext_modules = [
         CORE,