[pypy-commit] pypy default: issue1087: fix it by just checking the status of the lock to know

[arigo]

Author: Armin Rigo <arigo at tunes.org>
Branch: 
Changeset: r55531:4024e9cc54c6
Date: 2012-06-09 12:18 +0200
http://bitbucket.org/pypy/pypy/changeset/4024e9cc54c6/

Log:	issue1087: fix it by just checking the status of the lock to know if
	we can follow the fast path or not. With the GIL, there should be
	no race conditions.

diff --git a/pypy/module/imp/importing.py b/pypy/module/imp/importing.py
--- a/pypy/module/imp/importing.py
+++ b/pypy/module/imp/importing.py
@@ -263,7 +263,7 @@
                 w_mod = check_sys_modules_w(space, rel_modulename)
                 if w_mod is not None and space.is_w(w_mod, space.w_None):
                     # if we already find space.w_None, it means that we
-                    # already tried and failed and falled back to the
+                    # already tried and failed and fell back to the
                     # end of this function.
                     w_mod = None
                 else:
@@ -283,10 +283,16 @@
     return w_mod
 
 def absolute_import(space, modulename, baselevel, fromlist_w, tentative):
-    # Short path: check in sys.modules
-    w_mod = absolute_import_try(space, modulename, baselevel, fromlist_w)
-    if w_mod is not None and not space.is_w(w_mod, space.w_None):
-        return w_mod
+    # Short path: check in sys.modules, but only if there is no conflict
+    # on the import lock.  In the situation of 'import' statements
+    # inside tight loops, this should be true, and absolute_import_try()
+    # should be followed by the JIT and turned into not much code.  But
+    # if the import lock is currently held by another thread, then we
+    # have to wait, and so shouldn't use the fast path.
+    if not getimportlock(space).lock_held_by_someone_else():
+        w_mod = absolute_import_try(space, modulename, baselevel, fromlist_w)
+        if w_mod is not None and not space.is_w(w_mod, space.w_None):
+            return w_mod
     return absolute_import_with_lock(space, modulename, baselevel,
                                      fromlist_w, tentative)
 
@@ -741,6 +747,9 @@
         self.lockowner = None
         self.lockcounter = 0
 
+    def lock_held_by_someone_else(self):
+        return self.lockowner is not None and not self.lock_held()
+
     def lock_held(self):
         me = self.space.getexecutioncontext()   # used as thread ident
         return self.lockowner is me
diff --git a/pypy/module/imp/test/test_import.py b/pypy/module/imp/test/test_import.py
--- a/pypy/module/imp/test/test_import.py
+++ b/pypy/module/imp/test/test_import.py
@@ -1212,3 +1212,45 @@
         "objspace.usepycfiles": True,
         "objspace.lonepycfiles": True
     }
+
+
+class AppTestMultithreadedImp(object):
+    def setup_class(cls):
+        #if not conftest.option.runappdirect:
+        #    py.test.skip("meant as an -A test")
+        cls.space = gettestobjspace(usemodules=['thread', 'time'])
+        tmpfile = udir.join('test_multithreaded_imp.py')
+        tmpfile.write('''if 1:
+            x = 666
+            import time
+            for i in range(1000): time.sleep(0.001)
+            x = 42
+        ''')
+        cls.w_tmppath = cls.space.wrap(str(udir))
+
+    def test_multithreaded_import(self):
+        import sys, thread, time
+        oldpath = sys.path[:]
+        try:
+            sys.path.insert(0, self.tmppath)
+            got = []
+
+            def check():
+                import test_multithreaded_imp
+                got.append(getattr(test_multithreaded_imp, 'x', '?'))
+
+            for i in range(5):
+                thread.start_new_thread(check, ())
+
+            for n in range(100):
+                for i in range(105): time.sleep(0.001)
+                if len(got) == 5:
+                    break
+            else:
+                raise AssertionError("got %r so far but still waiting" %
+                                     (got,))
+
+            assert got == [42] * 5, got
+
+        finally:
+            sys.path[:] = oldpath