[pypy-svn] r46674 - in pypy/dist/pypy: lib module/__builtin__ module/thread module/thread/test

arigo at codespeak.net arigo at codespeak.net
Sun Sep 16 12:54:24 CEST 2007


Author: arigo
Date: Sun Sep 16 12:54:23 2007
New Revision: 46674

Added:
   pypy/dist/pypy/module/thread/importlock.py   (contents, props changed)
Modified:
   pypy/dist/pypy/lib/imp.py
   pypy/dist/pypy/module/__builtin__/importing.py
   pypy/dist/pypy/module/thread/__init__.py
   pypy/dist/pypy/module/thread/test/test_import_lock.py
Log:
* reimplement imp.{acquire,release}_lock(), as needed by lib/runpy.py.
* the "fine-grained locking" idea was too naive.  Reverted to regular
  locking around almost the whole __import__ hook.
* cope with this with a different workaround against prebuilt locks.



Modified: pypy/dist/pypy/lib/imp.py
==============================================================================
--- pypy/dist/pypy/lib/imp.py	(original)
+++ pypy/dist/pypy/lib/imp.py	Sun Sep 16 12:54:23 2007
@@ -140,3 +140,19 @@
 
 def is_frozen(name):
     return False
+
+# ____________________________________________________________
+
+try:
+    # PyPy-specific interface
+    from thread import _importlock_held    as lock_held
+    from thread import _importlock_acquire as acquire_lock
+    from thread import _importlock_release as release_lock
+except ImportError:
+    def lock_held():
+        """On platforms without threads, return False."""
+        return False
+    def acquire_lock():
+        """On platforms without threads, this function does nothing."""
+    def release_lock():
+        """On platforms without threads, this function does nothing."""

Modified: pypy/dist/pypy/module/__builtin__/importing.py
==============================================================================
--- pypy/dist/pypy/module/__builtin__/importing.py	(original)
+++ pypy/dist/pypy/module/__builtin__/importing.py	Sun Sep 16 12:54:23 2007
@@ -197,6 +197,15 @@
 importhook.unwrap_spec = [ObjSpace,str,W_Root,W_Root,W_Root]
 
 def absolute_import(space, modulename, baselevel, w_fromlist, tentative):
+    lock = getimportlock(space)
+    lock.acquire_lock()
+    try:
+        return _absolute_import(space, modulename, baselevel,
+                                w_fromlist, tentative)
+    finally:
+        lock.release_lock()
+
+def _absolute_import(space, modulename, baselevel, w_fromlist, tentative):
     w = space.wrap
     
     w_mod = None
@@ -246,45 +255,35 @@
         if not space.is_w(w_mod, space.w_None):
             return w_mod
     else:
-        w_mod = space.sys.getmodule(modulename) 
-        if w_mod is not None:
+        # Examin importhooks (PEP302) before doing the import
+        if w_path is not None:
+            w_loader  = find_module(space, w_modulename, w_path) 
+        else:
+            w_loader  = find_module(space, w_modulename, space.w_None)
+        if not space.is_w(w_loader, space.w_None):
+            w_mod = space.call_method(w_loader, "load_module", w_modulename)
+            #w_mod_ = check_sys_modules(space, w_modulename)
+            if w_mod is not None and w_parent is not None:
+                space.setattr(w_parent, w(partname), w_mod)
+
             return w_mod
 
-        lock = getimportlock(space)
-        acquired = lock.try_acquire_lock()
-        try:
-            # Examin importhooks (PEP302) before doing the import
-            if w_path is not None:
-                w_loader  = find_module(space, w_modulename, w_path) 
-            else:
-                w_loader  = find_module(space, w_modulename, space.w_None)
-            if not space.is_w(w_loader, space.w_None):
-                w_mod = space.call_method(w_loader, "load_module", w_modulename)
-                #w_mod_ = check_sys_modules(space, w_modulename)
-                if w_mod is not None and w_parent is not None:
-                    space.setattr(w_parent, w(partname), w_mod)
-
-                return w_mod
-            
-            
-            if w_path is not None:
-                for path in space.unpackiterable(w_path):
-                    dir = os.path.join(space.str_w(path), partname)
-                    if os.path.isdir(dir):
-                        fn = os.path.join(dir, '__init__')
-                        w_mod = try_import_mod(space, w_modulename, fn,
-                                               w_parent, w(partname),
-                                               pkgdir=dir)
-                        if w_mod is not None:
-                            return w_mod
-                    fn = os.path.join(space.str_w(path), partname)
-                    w_mod = try_import_mod(space, w_modulename, fn, w_parent,
-                                           w(partname))
+
+        if w_path is not None:
+            for path in space.unpackiterable(w_path):
+                dir = os.path.join(space.str_w(path), partname)
+                if os.path.isdir(dir):
+                    fn = os.path.join(dir, '__init__')
+                    w_mod = try_import_mod(space, w_modulename, fn,
+                                           w_parent, w(partname),
+                                           pkgdir=dir)
                     if w_mod is not None:
                         return w_mod
-        finally:
-            if acquired:
-                lock.release_lock()
+                fn = os.path.join(space.str_w(path), partname)
+                w_mod = try_import_mod(space, w_modulename, fn, w_parent,
+                                       w(partname))
+                if w_mod is not None:
+                    return w_mod
 
     if tentative:
         return None
@@ -300,43 +299,63 @@
 # as an attempt to avoid failure of 'from x import y' if module x is
 # still being executed in another thread.
 
+# This logic is tested in pypy.module.thread.test.test_import_lock.
+
 class ImportRLock:
-    _ann_seen = False
 
     def __init__(self, space):
         self.space = space
         self.lock = None
         self.lockowner = None
+        self.lockcounter = 0
 
-    def _freeze_(self):
-        # remove the lock object, which will be created again as need at
-        # run-time.
-        self.lock = None
-        assert self.lockowner is None
-        self._ann_seen = True
-        return False
+    def lock_held(self):
+        me = self.space.getexecutioncontext()   # used as thread ident
+        return self.lockowner is me
 
-    def try_acquire_lock(self):
+    def _can_have_lock(self):
+        # hack: we can't have self.lock != None during translation,
+        # because prebuilt lock objects are not allowed.  In this
+        # special situation we just don't lock at all (translation is
+        # not multithreaded anyway).
+        if we_are_translated():
+            return True     # we need a lock at run-time
+        elif self.space.config.translating:
+            assert self.lock is None
+            return False
+        else:
+            return True     # in py.py
+
+    def acquire_lock(self):
         # this function runs with the GIL acquired so there is no race
         # condition in the creation of the lock
         if self.lock is None:
-            # sanity-check: imports should not occur after the
-            # ImportRLock has been frozen (and self.lock thrown away)
-            # during annotation.  They can occur either before, or in
-            # the translated code.
-            if not we_are_translated():
-                assert not self._ann_seen
+            if not self._can_have_lock():
+                return
             self.lock = self.space.allocate_lock()
         me = self.space.getexecutioncontext()   # used as thread ident
         if self.lockowner is me:
-            return False    # already acquired by the current thread
-        self.lock.acquire(True)
-        self.lockowner = me
-        return True
+            pass    # already acquired by the current thread
+        else:
+            self.lock.acquire(True)
+            assert self.lockowner is None
+            assert self.lockcounter == 0
+            self.lockowner = me
+        self.lockcounter += 1
 
     def release_lock(self):
-        self.lockowner = None
-        self.lock.release()
+        me = self.space.getexecutioncontext()   # used as thread ident
+        if self.lockowner is not me:
+            if not self._can_have_lock():
+                return
+            space = self.space
+            raise OperationError(space.w_RuntimeError,
+                                 space.wrap("not holding the import lock"))
+        assert self.lockcounter > 0
+        self.lockcounter -= 1
+        if self.lockcounter == 0:
+            self.lockowner = None
+            self.lock.release()
 
 def getimportlock(space):
     return space.fromcache(ImportRLock)

Modified: pypy/dist/pypy/module/thread/__init__.py
==============================================================================
--- pypy/dist/pypy/module/thread/__init__.py	(original)
+++ pypy/dist/pypy/module/thread/__init__.py	Sun Sep 16 12:54:23 2007
@@ -17,6 +17,11 @@
         'allocate':               'os_lock.allocate_lock',  # obsolete synonym
         'LockType':               'os_lock.getlocktype(space)',
         '_local':                 'os_local.getlocaltype(space)',
+
+        # custom interface for the 'imp' module
+        '_importlock_held':       'importlock.held',
+        '_importlock_acquire':    'importlock.acquire',
+        '_importlock_release':    'importlock.release',
     }
 
     def __init__(self, space, *args):

Added: pypy/dist/pypy/module/thread/importlock.py
==============================================================================
--- (empty file)
+++ pypy/dist/pypy/module/thread/importlock.py	Sun Sep 16 12:54:23 2007
@@ -0,0 +1,14 @@
+# link between the app-level 'imp' module and '__builtin__.importing'.
+# XXX maybe this should go together with 'imp.py' and the interp-level
+# import logic into a MixedModule.
+
+from pypy.module.__builtin__ import importing
+
+def held(space):
+    return space.wrap(importing.getimportlock(space).lock_held())
+
+def acquire(space):
+    importing.getimportlock(space).acquire_lock()
+
+def release(space):
+    importing.getimportlock(space).release_lock()

Modified: pypy/dist/pypy/module/thread/test/test_import_lock.py
==============================================================================
--- pypy/dist/pypy/module/thread/test/test_import_lock.py	(original)
+++ pypy/dist/pypy/module/thread/test/test_import_lock.py	Sun Sep 16 12:54:23 2007
@@ -10,12 +10,16 @@
         cls.w_tmpdir = cls.space.wrap(tmpdir)
 
     def test_import_lock(self):
-        import thread
+        import thread, imp
+        assert not imp.lock_held()
         done = []
         def f():
+            print '[ENTER %d]' % i
             from imghdr import testall
+            print '[LEAVE %d]' % i
             done.append(1)
         for i in range(5):
+            print '[RUN %d]' % i
             thread.start_new_thread(f, ())
         self.waitfor(lambda: len(done) == 5)
         assert len(done) == 5
@@ -24,23 +28,34 @@
         import thread
         import re      # -> causes nested imports
 
-    def test_no_lock_for_reimporting(self):
-        # CPython deadlocks in this situation; in our case the property
-        # of not requiring the import lock for already-imported modules
-        # is useful for translation, to avoid needing a prebuilt import
-        # lock object.
-        import os, sys
-        f = open(os.path.join(self.tmpdir, 'foobaz1.py'), 'w')
-        print >> f, """if 1:
-            import thread
-            lock = thread.allocate_lock()
-            lock.acquire()
-            def f():
-                import sys
-                lock.release()
-            thread.start_new_thread(f, ())
-            lock.acquire()
-        """
-        f.close()
-        sys.path.insert(0, self.tmpdir)
-        import foobaz1
+    def test_manual_locking(self):
+        import thread, os, imp, time, sys
+        f = open(os.path.join(self.tmpdir, 'foobaz2.py'), 'w')
+        f.close()   # empty
+        done = []
+        def f():
+            sys.path.insert(0, self.tmpdir)
+            import foobaz2
+            p = sys.path.pop(0)
+            assert p == self.tmpdir
+            done.append(1)
+        assert not imp.lock_held()
+        imp.acquire_lock()
+        assert imp.lock_held()
+        thread.start_new_thread(f, ())
+        time.sleep(0.9)
+        assert not done
+        assert imp.lock_held()
+        # check that it's a recursive lock
+        imp.acquire_lock()
+        assert imp.lock_held()
+        imp.acquire_lock()
+        assert imp.lock_held()
+        imp.release_lock()
+        assert imp.lock_held()
+        imp.release_lock()
+        assert imp.lock_held()
+        imp.release_lock()
+        assert not imp.lock_held()
+        self.waitfor(lambda: done)
+        assert done



More information about the Pypy-commit mailing list