Use weakref to deal with GC issues, rather than trying to clean up ou…

…rselves

This prevents the error messages in cocotb#650.

It also:
* Fixes a bug where `coro.join()` returns the result the first time, but `None` on subsequent times
* Replaces four ways to spell join (`coro._join`, `coro.join()`, `_Join(coro)`, `Join(coro)`) with two (`coro.join()`, `Join(coro)`)

cocotb/decorators.py

@@ -37,7 +37,7 @@
 
 import cocotb
 from cocotb.log import SimLog
-from cocotb.triggers import _Join, PythonTrigger, Timer, Event, NullTrigger, Join
+from cocotb.triggers import Join, PythonTrigger, Timer, Event, NullTrigger
 from cocotb.result import (TestComplete, TestError, TestFailure, TestSuccess,
                            ReturnValue, raise_error, ExternalException)
 from cocotb.utils import get_sim_time
@@ -94,7 +94,6 @@ def __init__(self, inst, parent):
         self._started = False
         self._finished = False
         self._callbacks = []
-        self._join = _Join(self)
         self._parent = parent
         self.__doc__ = parent._func.__doc__
         self.module = parent._func.__module__
@@ -159,10 +158,7 @@ def _finished_cb(self):
 
     def join(self):
         """Return a trigger that will fire when the wrapped coroutine exits"""
-        if self._finished:
-            return NullTrigger()
-        else:
-            return self._join
+        return Join(self)
 
     def has_started(self):
         return self._started

cocotb/scheduler.py

@@ -67,7 +67,7 @@
 import cocotb
 import cocotb.decorators
 from cocotb.triggers import (Trigger, GPITrigger, Timer, ReadOnly, PythonTrigger,
-                             _NextTimeStep, _ReadWrite, Event, NullTrigger)
+                             _NextTimeStep, _ReadWrite, Event, Join)
 from cocotb.log import SimLog
 from cocotb.result import (TestComplete, TestError, ReturnValue, raise_error,
                            create_error, ExternalException)
@@ -450,11 +450,9 @@ def unschedule(self, coro):
                 del self._trigger2coros[trigger]
         del self._coro2triggers[coro]
 
-        if coro._join in self._trigger2coros:
-            self._pending_triggers.append(coro._join)
+        if Join(coro) in self._trigger2coros:
+            self._pending_triggers.append(Join(coro))
 
-        # Remove references to allow GC to clean up
-        del coro._join
 
     def save_write(self, handle, value):
         if self._mode == Scheduler._MODE_READONLY:

cocotb/triggers.py

@@ -29,6 +29,7 @@
     A collections of triggers which a testbench can 'yield'
 """
 import os
+import weakref
 
 # For autodocumentation don't need the extension modules
 if "SPHINX_BUILD" in os.environ:
@@ -520,26 +521,37 @@ def prime(self, callback):
         callback(self)
 
 
-class _Join(PythonTrigger):
+class Join(PythonTrigger):
     """
     Join a coroutine, firing when it exits
     """
+    # Ensure that each coroutine has at most one join trigger.
+    # Using a weak dictionary ensures we don't create a reference cycle
+    _instances = weakref.WeakValueDictionary()
+
+    def __new__(cls, coroutine):
+        # find the existing instance, if possible - else create a new one
+        try:
+            return cls._instances[coroutine]
+        except KeyError:
+            instance = super(Join, cls).__new__(cls)
+            cls._instances[coroutine] = instance
+            return instance
+
     def __init__(self, coroutine):
-        PythonTrigger.__init__(self)
+        super(Join, self).__init__()
         self._coroutine = coroutine
         self.pass_retval = True
 
     @property
     def retval(self):
         return self._coroutine.retval
 
-    # def prime(self, callback):
-        # """Register our calback for when the coroutine exits"""
-        # Trigger.prime(self)
+    def prime(self, callback):
+        if self._coroutine._finished:
+            callback(self)
+        else:
+            super(Join, self).prime(callback)
 
     def __str__(self):
         return self.__class__.__name__ + "(%s)" % self._coroutine.__name__
-
-
-def Join(coro):
-    return coro._join

tests/test_cases/test_cocotb/test_cocotb.py

@@ -712,6 +712,58 @@ def test_binary_value_compat(dut):
     yield Timer(100)  # Make it do something with time
 
 
+@cocotb.test()
+def join_finished(dut):
+    """
+    Test that joining a coroutine that has already been joined gives
+    the same result as it did the first time.
+    """
+
+    retval = None
+
+    @cocotb.coroutine
+    def some_coro():
+        yield Timer(1)
+        raise ReturnValue(retval)
+
+    coro = cocotb.fork(some_coro())
+
+    retval = 1
+    x = yield coro.join()
+    assert x == 1
+
+    # joining the second time should give the same result.
+    # we chage retval here to prove it does not run again
+    retval = 2
+    x = yield coro.join()
+    assert x == 1
+
+
+@cocotb.test()
+def test_kill_twice(dut):
+    """
+    Test that killing a coroutine that has already been killed does not crash
+    """
+    clk_gen = cocotb.fork(Clock(dut.clk, 100).start())
+    yield Timer(1)
+    clk_gen.kill()
+    yield Timer(1)
+    clk_gen.kill()
+
+
+@cocotb.test()
+def test_join_identity(dut):
+    """
+    Test that Join() returns the same object each time
+    """
+    clk_gen = cocotb.fork(Clock(dut.clk, 100).start())
+
+    assert Join(clk_gen) is Join(clk_gen)
+    yield Timer(1)
+    clk_gen.kill()
+
+
+
 if sys.version_info[:2] >= (3, 3):
     # this would be a syntax error in older python, so we do the whole
     # thing inside exec