[pypy-svn] r46568 - in pypy/dist/pypy: module/_socket module/thread module/thread/test rlib rpython/lltypesystem

arigo at codespeak.net arigo at codespeak.net
Fri Sep 14 11:42:27 CEST 2007


Author: arigo
Date: Fri Sep 14 11:42:25 2007
New Revision: 46568

Modified:
   pypy/dist/pypy/module/_socket/interp_func.py
   pypy/dist/pypy/module/_socket/interp_socket.py
   pypy/dist/pypy/module/thread/gil.py
   pypy/dist/pypy/module/thread/ll_thread.py
   pypy/dist/pypy/module/thread/os_lock.py
   pypy/dist/pypy/module/thread/test/test_ll_thread.py
   pypy/dist/pypy/module/thread/test/test_lock.py
   pypy/dist/pypy/rlib/objectmodel.py
   pypy/dist/pypy/rpython/lltypesystem/rffi.py
Log:
Implement automatic GIL release around external function calls.
Remove the manual GIL releases where I found them:
* module/_socket
* lock.acquire()


Modified: pypy/dist/pypy/module/_socket/interp_func.py
==============================================================================
--- pypy/dist/pypy/module/_socket/interp_func.py	(original)
+++ pypy/dist/pypy/module/_socket/interp_func.py	Fri Sep 14 11:42:25 2007
@@ -10,12 +10,7 @@
     Return the current host name.
     """
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            res = rsocket.gethostname()
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        res = rsocket.gethostname()
     except SocketError, e:
         raise converted_error(space, e)
     return space.wrap(res)
@@ -27,12 +22,7 @@
     Return the IP address (a string of the form '255.255.255.255') for a host.
     """
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            addr = rsocket.gethostbyname(hostname)
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        addr = rsocket.gethostbyname(hostname)
         ip = addr.get_host()
     except SocketError, e:
         raise converted_error(space, e)
@@ -53,12 +43,7 @@
     for a host.  The host argument is a string giving a host name or IP number.
     """
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            res = rsocket.gethostbyname_ex(host)
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        res = rsocket.gethostbyname_ex(host)
     except SocketError, e:
         raise converted_error(space, e)
     return common_wrapgethost(space, res)
@@ -71,12 +56,7 @@
     for a host.  The host argument is a string giving a host name or IP number.
     """
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            res = rsocket.gethostbyaddr(host)
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        res = rsocket.gethostbyaddr(host)
     except SocketError, e:
         raise converted_error(space, e)
     return common_wrapgethost(space, res)
@@ -94,12 +74,7 @@
     else:
         proto = space.str_w(w_proto)
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            port = rsocket.getservbyname(name, proto)
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        port = rsocket.getservbyname(name, proto)
     except SocketError, e:
         raise converted_error(space, e)
     return space.wrap(port)
@@ -117,12 +92,7 @@
     else:
         proto = space.str_w(w_proto)
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            service = rsocket.getservbyport(port, proto)
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        service = rsocket.getservbyport(port, proto)
     except SocketError, e:
         raise converted_error(space, e)
     return space.wrap(service)
@@ -134,12 +104,7 @@
     Return the protocol number for the named protocol.  (Rarely used.)
     """
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            proto = rsocket.getprotobyname(name)
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        proto = rsocket.getprotobyname(name)
     except SocketError, e:
         raise converted_error(space, e)
     return space.wrap(proto)
@@ -150,14 +115,8 @@
 
     Get host and port for a sockaddr."""
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            addr = rsocket.ipaddr_from_object(space, w_sockaddr)
-            host, servport = rsocket.getnameinfo(addr, flags)
-        finally:
-            if GIL is not None: GIL.acquire(True)
-            
+        addr = rsocket.ipaddr_from_object(space, w_sockaddr)
+        host, servport = rsocket.getnameinfo(addr, flags)
     except SocketError, e:
         raise converted_error(space, e)
     return space.newtuple([space.wrap(host), space.wrap(servport)])
@@ -327,14 +286,8 @@
         raise OperationError(space.w_TypeError,
                              space.wrap("Int or String expected"))
     try:
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            lst = rsocket.getaddrinfo(host, port, family, socktype,
-                                      proto, flags)
-        finally:
-            if GIL is not None: GIL.acquire(True)
-            
+        lst = rsocket.getaddrinfo(host, port, family, socktype,
+                                  proto, flags)
     except SocketError, e:
         raise converted_error(space, e)
     lst1 = [space.newtuple([space.wrap(family),

Modified: pypy/dist/pypy/module/_socket/interp_socket.py
==============================================================================
--- pypy/dist/pypy/module/_socket/interp_socket.py	(original)
+++ pypy/dist/pypy/module/_socket/interp_socket.py	Fri Sep 14 11:42:25 2007
@@ -30,12 +30,7 @@
         info is a pair (hostaddr, port).
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                sock, addr = self.accept(W_RSocket)
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            sock, addr = self.accept(W_RSocket)
             return space.newtuple([space.wrap(sock),
                                    addr.as_object(space)])
         except SocketError, e:
@@ -50,12 +45,7 @@
         sockets the address is a tuple (ifname, proto [,pkttype [,hatype]])
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                self.bind(self.addr_from_object(space, w_addr))
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            self.bind(self.addr_from_object(space, w_addr))
         except SocketError, e:
             raise converted_error(space, e)
     bind_w.unwrap_spec = ['self', ObjSpace, W_Root]
@@ -66,12 +56,7 @@
         Close the socket.  It cannot be used after this call.
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                self.close()
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            self.close()
         except SocketError, e:
             raise converted_error(space, e)
     close_w.unwrap_spec = ['self', ObjSpace]
@@ -83,12 +68,7 @@
         is a pair (host, port).
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                self.connect(self.addr_from_object(space, w_addr))
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            self.connect(self.addr_from_object(space, w_addr))
         except SocketError, e:
             raise converted_error(space, e)
         except TypeError, e:
@@ -102,12 +82,7 @@
         This is like connect(address), but returns an error code (the errno value)
         instead of raising an exception when an error occurs.
         """
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None: GIL.release()
-        try:
-            error = self.connect_ex(self.addr_from_object(space, w_addr))
-        finally:
-            if GIL is not None: GIL.acquire(True)
+        error = self.connect_ex(self.addr_from_object(space, w_addr))
         return space.wrap(error)
     connect_ex_w.unwrap_spec = ['self', ObjSpace, W_Root]
 
@@ -137,12 +112,7 @@
         info is a pair (hostaddr, port).
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                addr = self.getpeername()
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            addr = self.getpeername()
             return addr.as_object(space)
         except SocketError, e:
             raise converted_error(space, e)
@@ -155,12 +125,7 @@
         info is a pair (hostaddr, port).
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                addr = self.getsockname()
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            addr = self.getsockname()
             return addr.as_object(space)
         except SocketError, e:
             raise converted_error(space, e)
@@ -202,12 +167,7 @@
         will allow before refusing new connections.
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                self.listen(backlog)
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            self.listen(backlog)
         except SocketError, e:
             raise converted_error(space, e)
     listen_w.unwrap_spec = ['self', ObjSpace, int]
@@ -232,13 +192,7 @@
         the remote end is closed and all data is read, return the empty string.
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                data = self.recv(buffersize, flags)
-            finally:
-                if GIL is not None: GIL.acquire(True)
-                
+            data = self.recv(buffersize, flags)
         except SocketError, e:
             raise converted_error(space, e)
         return space.wrap(data)
@@ -250,12 +204,7 @@
         Like recv(buffersize, flags) but also return the sender's address info.
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                data, addr = self.recvfrom(buffersize, flags)
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            data, addr = self.recvfrom(buffersize, flags)
             if addr:
                 w_addr = addr.as_object(space)
             else:
@@ -274,12 +223,7 @@
         """
         data = coerce_to_str_w(space, w_data)
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                count = self.send(data, flags)
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            count = self.send(data, flags)
         except SocketError, e:
             raise converted_error(space, e)
         return space.wrap(count)
@@ -295,12 +239,7 @@
         """
         data = coerce_to_str_w(space, w_data)
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                count = self.sendall(data, flags)
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            count = self.sendall(data, flags)
         except SocketError, e:
             raise converted_error(space, e)
     sendall_w.unwrap_spec = ['self', ObjSpace, W_Root, int]
@@ -321,13 +260,8 @@
             flags = space.int_w(w_param2)
             w_addr = w_param3
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                addr = self.addr_from_object(space, w_addr)
-                count = self.sendto(data, flags, addr)
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            addr = self.addr_from_object(space, w_addr)
+            count = self.sendto(data, flags, addr)
         except SocketError, e:
             raise converted_error(space, e)
         return space.wrap(count)
@@ -391,12 +325,7 @@
         (flag == SHUT_RDWR).
         """
         try:
-            GIL = space.threadlocals.getGIL()
-            if GIL is not None: GIL.release()
-            try:
-                self.shutdown(how)
-            finally:
-                if GIL is not None: GIL.acquire(True)
+            self.shutdown(how)
         except SocketError, e:
             raise converted_error(space, e)
     shutdown_w.unwrap_spec = ['self', ObjSpace, int]

Modified: pypy/dist/pypy/module/thread/gil.py
==============================================================================
--- pypy/dist/pypy/module/thread/gil.py	(original)
+++ pypy/dist/pypy/module/thread/gil.py	Fri Sep 14 11:42:25 2007
@@ -10,6 +10,7 @@
 from pypy.module.thread import ll_thread as thread
 from pypy.interpreter.miscutils import Action
 from pypy.module.thread.threadlocals import OSThreadLocals
+from pypy.rlib.objectmodel import invoke_around_extcall
 
 class GILThreadLocals(OSThreadLocals):
     """A version of OSThreadLocals that enforces a GIL."""
@@ -18,13 +19,25 @@
     def setup_threads(self, space):
         """Enable threads in the object space, if they haven't already been."""
         if self.GIL is None:
-            self.GIL = thread.allocate_lock()
+            self.GIL = thread.allocate_lock_NOAUTO()
             self.enter_thread(space)   # setup the main thread
             # add the GIL-releasing callback as an action on the space
             space.pending_actions.append(GILReleaseAction(self))
-            return True
+            result = True
         else:
-            return False      # already set up
+            result = False      # already set up
+
+        # add the GIL-releasing callback around external function calls.
+        #
+        # XXX we assume a single space, but this is not quite true during
+        # testing; for example, if you run the whole of test_lock you get
+        # a deadlock caused by the first test's space being reused by
+        # test_lock_again after the global state was cleared by
+        # test_compile_lock.  As a workaround, we repatch these global
+        # fields systematically.
+        spacestate.GIL = self.GIL
+        invoke_around_extcall(before_external_call, after_external_call)
+        return result
 
     def enter_thread(self, space):
         "Notification that the current thread is just starting: grab the GIL."
@@ -60,3 +73,14 @@
 
     def perform(self):
         self.threadlocals.yield_thread()
+
+
+class SpaceState:
+    pass
+spacestate = SpaceState()
+
+def before_external_call():
+    spacestate.GIL.release()
+
+def after_external_call():
+    spacestate.GIL.acquire(True)

Modified: pypy/dist/pypy/module/thread/ll_thread.py
==============================================================================
--- pypy/dist/pypy/module/thread/ll_thread.py	(original)
+++ pypy/dist/pypy/module/thread/ll_thread.py	Fri Sep 14 11:42:25 2007
@@ -10,7 +10,7 @@
 from pypy.rpython.extregistry import ExtRegistryEntry
 from pypy.annotation import model as annmodel
 from pypy.rpython.lltypesystem.lltype import typeOf
-from pypy.rlib.objectmodel import we_are_translated
+from pypy.rlib.objectmodel import debug_assert
 from pypy.rlib.nonconst import NonConstant
 
 class error(Exception):
@@ -30,9 +30,10 @@
     return str(pypydir.join('_cache', modname)) + '.so'
 libraries = [setup_thread_so()]
 
-def llexternal(name, args, result):
+def llexternal(name, args, result, **kwds):
     return rffi.llexternal(name, args, result, includes=includes,
-                           libraries=libraries, include_dirs=[str(c_dir)])
+                           libraries=libraries, include_dirs=[str(c_dir)],
+                           **kwds)
 
 CALLBACK = lltype.Ptr(lltype.FuncType([rffi.VOIDP], rffi.VOIDP))
 c_thread_start = llexternal('RPyThreadStart', [CALLBACK, rffi.VOIDP], rffi.INT)
@@ -44,8 +45,18 @@
 c_thread_acquirelock = llexternal('RPyThreadAcquireLock', [TLOCKP, rffi.INT],
                                   rffi.INT)
 c_thread_releaselock = llexternal('RPyThreadReleaseLock', [TLOCKP], lltype.Void)
-c_thread_fused_releaseacquirelock = llexternal(
-    'RPyThreadFusedReleaseAcquireLock', [TLOCKP], lltype.Void)
+
+# another set of functions, this time in versions that don't cause the
+# GIL to be released.  To use to handle the GIL lock itself.
+c_thread_acquirelock_NOAUTO = llexternal('RPyThreadAcquireLock',
+                                         [TLOCKP, rffi.INT], rffi.INT,
+                                         sandboxsafe=True)
+c_thread_releaselock_NOAUTO = llexternal('RPyThreadReleaseLock',
+                                         [TLOCKP], lltype.Void,
+                                         sandboxsafe=True)
+c_thread_fused_releaseacquirelock_NOAUTO = llexternal(
+     'RPyThreadFusedReleaseAcquireLock', [TLOCKP], lltype.Void,
+                                         sandboxsafe=True)
 
 def allocate_lock():
     ll_lock = lltype.malloc(TLOCKP.TO, flavor='raw')
@@ -55,6 +66,14 @@
         raise error("out of resources")
     return Lock(ll_lock)
 
+def allocate_lock_NOAUTO():
+    ll_lock = lltype.malloc(TLOCKP.TO, flavor='raw')
+    res = c_thread_lock_init(ll_lock)
+    if res == -1:
+        lltype.free(ll_lock, flavor='raw')
+        raise error("out of resources")
+    return Lock_NOAUTO(ll_lock)
+
 def _start_new_thread(x, y):
     return thread.start_new_thread(x, (y,))
 
@@ -123,14 +142,26 @@
         else:
             c_thread_releaselock(self._lock)
 
+    def __del__(self):
+        lltype.free(self._lock, flavor='raw')
+
+class Lock_NOAUTO(object):
+    """A special lock that doesn't cause the GIL to be released when
+    we try to acquire it.  Used for the GIL itself."""
+
+    def __init__(self, ll_lock):
+        self._lock = ll_lock
+
+    def acquire(self, flag):
+        return bool(c_thread_acquirelock_NOAUTO(self._lock, int(flag)))
+
+    def release(self):
+        debug_assert(not self.acquire(False), "Lock_NOAUTO was not held!")
+        c_thread_releaselock_NOAUTO(self._lock)
+
     def fused_release_acquire(self):
-        # Sanity check: the lock must be locked
-        if self.acquire(False):
-            c_thread_releaselock(self._lock)
-            raise error(NonConstant("bad lock"))
-        else:
-            c_thread_fused_releaseacquirelock(self._lock)
+        debug_assert(not self.acquire(False), "Lock_NOAUTO was not held!")
+        c_thread_fused_releaseacquirelock_NOAUTO(self._lock)
 
     def __del__(self):
         lltype.free(self._lock, flavor='raw')
-

Modified: pypy/dist/pypy/module/thread/os_lock.py
==============================================================================
--- pypy/dist/pypy/module/thread/os_lock.py	(original)
+++ pypy/dist/pypy/module/thread/os_lock.py	Fri Sep 14 11:42:25 2007
@@ -39,15 +39,8 @@
 With an argument, this will only block if the argument is true,
 and the return value reflects whether the lock is acquired.
 The blocking operation is not interruptible."""
-        # XXX Usage of threadlocals.GIL in this function is considered hackish.
-        #     Ideally, all GIL knowledge should be in gil.py.
         mylock = self.lock
-        GIL = space.threadlocals.getGIL()
-        if GIL is not None:
-            GIL.release()
         result = mylock.acquire(bool(waitflag))
-        if GIL is not None:
-            GIL.acquire(True)
         return space.newbool(result)
 
     def descr_lock_release(self, space):

Modified: pypy/dist/pypy/module/thread/test/test_ll_thread.py
==============================================================================
--- pypy/dist/pypy/module/thread/test/test_ll_thread.py	(original)
+++ pypy/dist/pypy/module/thread/test/test_ll_thread.py	Fri Sep 14 11:42:25 2007
@@ -25,17 +25,14 @@
         py.test.fail("Did not raise")
 
 def test_fused():
-    l = allocate_lock()
-    try:
-        l.fused_release_acquire()
-    except error:
-        pass
-    else:
-        py.test.fail("Did not raise")
+    l = allocate_lock_NOAUTO()
     l.acquire(True)
     l.fused_release_acquire()
     could_acquire_again = l.acquire(False)
     assert not could_acquire_again
+    l.release()
+    could_acquire_again = l.acquire(False)
+    assert could_acquire_again
 
 def test_start_new_thread():
     import time

Modified: pypy/dist/pypy/module/thread/test/test_lock.py
==============================================================================
--- pypy/dist/pypy/module/thread/test/test_lock.py	(original)
+++ pypy/dist/pypy/module/thread/test/test_lock.py	Fri Sep 14 11:42:25 2007
@@ -1,4 +1,5 @@
 from pypy.module.thread.test.support import GenericTestThread
+from pypy.translator.c.test.test_genc import compile
 
 
 class AppTestLock(GenericTestThread):
@@ -27,3 +28,29 @@
         lock.acquire()
         assert lock.locked() is True
         assert feedback == [42]
+
+
+def test_compile_lock():
+    from pypy.rlib import rgc
+    from pypy.module.thread.ll_thread import allocate_lock
+    def g():
+        l = allocate_lock()
+        ok1 = l.acquire(True)
+        ok2 = l.acquire(False)
+        l.release()
+        ok3 = l.acquire(False)
+        res = ok1 and not ok2 and ok3
+        return res
+    g.dont_inline = True
+    def f():
+        res = g()
+        # the lock must have been freed by now - we use refcounting
+        return res
+    fn = compile(f, [], gcpolicy='ref')
+    res = fn()
+    assert res
+
+
+class AppTestLockAgain(GenericTestThread):
+    # test it at app-level again to detect strange interactions
+    test_lock_again = AppTestLock.test_lock.im_func

Modified: pypy/dist/pypy/rlib/objectmodel.py
==============================================================================
--- pypy/dist/pypy/rlib/objectmodel.py	(original)
+++ pypy/dist/pypy/rlib/objectmodel.py	Fri Sep 14 11:42:25 2007
@@ -152,12 +152,12 @@
     """Call before() before any external function call, and after() after.
     At the moment only one pair before()/after() can be registered at a time.
     """
-    if not we_are_translated():
-        raise NotImplementedError("only works after rtyping")
+    # NOTE: the hooks are cleared during translation!  To be effective
+    # in a compiled program they must be set at run-time.
     from pypy.rpython.lltypesystem import rffi
     from pypy.rpython.annlowlevel import llhelper
-    rffi._ll_invoke_around_extcall(llhelper(rffi.AroundFnPtr, before),
-                                   llhelper(rffi.AroundFnPtr, after))
+    rffi.aroundstate.before = llhelper(rffi.AroundFnPtr, before)
+    rffi.aroundstate.after  = llhelper(rffi.AroundFnPtr, after)
 
 
 class UnboxedValue(object):

Modified: pypy/dist/pypy/rpython/lltypesystem/rffi.py
==============================================================================
--- pypy/dist/pypy/rpython/lltypesystem/rffi.py	(original)
+++ pypy/dist/pypy/rpython/lltypesystem/rffi.py	Fri Sep 14 11:42:25 2007
@@ -100,13 +100,13 @@
     return func_with_new_name(wrapper, name)
 
 AroundFnPtr = lltype.Ptr(lltype.FuncType([], lltype.Void))
-AroundState = lltype.Struct('AroundState',
-                             ('before', AroundFnPtr),
-                             ('after',  AroundFnPtr))
-aroundstate = lltype.malloc(AroundState, immortal=True, zero=True)
-def _ll_invoke_around_extcall(before, after):
-    aroundstate.before = before
-    aroundstate.after = after
+class AroundState:
+    def _freeze_(self):
+        self.before = lltype.nullptr(AroundFnPtr.TO)
+        self.after  = lltype.nullptr(AroundFnPtr.TO)
+        return False
+aroundstate = AroundState()
+aroundstate._freeze_()
 
 # ____________________________________________________________
 



More information about the Pypy-commit mailing list