classification
Title: Add circular imports test
Type: behavior Stage: resolved
Components: Tests Versions: Python 3.1, Python 3.2
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: brett.cannon, ncoghlan, pitrou
Priority: normal Keywords: patch

Created on 2010-08-21 18:43 by pitrou, last changed 2010-08-22 20:48 by pitrou. This issue is now closed.

Files
File name Uploaded Description Edit
circulartest.patch pitrou, 2010-08-21 18:43
Messages (5)
msg114541 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-21 18:43
Here is a test for multi-threaded circular imports (in order to exercise the import lock), following a comment by Graham Dumpleton on #9260.
msg114652 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-08-22 09:16
It would be good if the test timed out rather than deadlocking in the face of a broken import lock.

I suggest:
1. Make the two test threads daemon threads
2. Specify a timeout to the join() calls
3. Use self.assertFalse(t1.is_alive()) and self.assertFalse(t2.is_alive()) to check the calls actually finished

For your #9260 testing, this looks like it may be a little probabilistic. I think you can make the deadlock a near certainty by defining your test modules and threads as follows:

# Create a circular import structure: A -> C -> B -> D -> A
circular_imports_modules = {
    'A': """if 1:
        import ev
        ev.evA.wait()
        import time
        time.sleep(%(delay)s)
        x = 'a'
        import C
        """,
    'B': """if 1:
        import ev
        ev.evB.wait()
        import time
        time.sleep(%(delay)s)
        x = 'b'
        import D
        """,
    'C': """import B""",
    'D': """import A""",
    'ev': """if 1:
         import threading;
         evA = threading.Event()
         evB = threading.Event()
         """,
}

        def import_ab():
            import ev
            ev.evB.set()
            import A
            results.append(getattr(A, 'x', None))
        def import_ba():
            import ev
            ev.evA.set()
            import B
            results.append(getattr(B, 'x', None))


(I've done a trick along these lines before, and the above doesn't look quite right. Maybe the details will come back to me if I sit on it for a while)
msg114656 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-22 10:07
> It would be good if the test timed out rather than deadlocking in the
> face of a broken import lock.

I could do that indeed. I'm not sure it would be much better, though, because the lock will still be held and posterior tests may freeze mysteriously.

> For your #9260 testing, this looks like it may be a little probabilistic

It always freezes here (with the per-module lock). I guess on modern machines, 0.5s is more than enough to launch a thread and parse a nearly empty Python module.

I should add a comment explaining a purpose of the test, though.
msg114674 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2010-08-22 12:29
Both good points - don't worry about it then. +1 to add to the test suite as-is.
msg114705 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2010-08-22 20:48
Thank you Nick! I've committed the patch in r84268 (py3k) and r84269 (3.1).
History
Date User Action Args
2010-08-22 20:48:07pitrousetstatus: open -> closed
versions: + Python 3.1
messages: + msg114705

resolution: fixed
stage: patch review -> resolved
2010-08-22 12:29:26ncoghlansetmessages: + msg114674
2010-08-22 10:07:11pitrousetmessages: + msg114656
2010-08-22 09:16:14ncoghlansetmessages: + msg114652
2010-08-21 18:43:58pitroucreate