asyncio: clarify strong refs for run_coroutine_threadsafe

We added some code in https://github.com/spesmilo/electrum/commit/0b3a28358681a0fe101bd6fc3feb5ff981fb68ce
to explicitly hold strong refs for all tasks/futures. At the time I was uncertain if that also solves
GC issues with asyncio.run_coroutine_threadsafe.
ref https://github.com/spesmilo/electrum/pull/9608#issuecomment-2703681663

Looks like it does. run_coroutine_threadsafe *is* going through the custom task factory.
See the unit test.
The somewhat confusing thing is that we need a few event loop iterations for the task factory to run,
due to how run_coroutine_threadsafe is implemented. And also, the task that we will hold as strong ref
in the global set is not the concurrent.futures.Future that run_coroutine_threadsafe returns.

So this commit simply "fixes" the unit test so that it showcases this, and removes related, older, plumbing
from util.py that we now know is no longer needed because of this.

electrum/util.py

@@ -1684,7 +1684,7 @@ def _set_custom_task_factory(loop: asyncio.AbstractEventLoop):
     - "asyncio.create_task"
     - "loop.create_task"
     - "asyncio.ensure_future"
-    - what about "asyncio.run_coroutine_threadsafe"? not sure if that is safe.
+    - "asyncio.run_coroutine_threadsafe"
 
     related:
         - https://bugs.python.org/issue44665
@@ -1858,7 +1858,6 @@ class CallbackManager(Logger):
         Logger.__init__(self)
         self.callback_lock = threading.Lock()
         self.callbacks = defaultdict(list)      # note: needs self.callback_lock
-        self._running_cb_futs = set()
 
     def register_callback(self, func, events):
         with self.callback_lock:
@@ -1883,11 +1882,8 @@ class CallbackManager(Logger):
         for callback in callbacks:
             if asyncio.iscoroutinefunction(callback):  # async cb
                 fut = asyncio.run_coroutine_threadsafe(callback(*args), loop)
-                # keep strong references around to avoid GC issues:
-                self._running_cb_futs.add(fut)
                 def on_done(fut_: concurrent.futures.Future):
                     assert fut_.done()
-                    self._running_cb_futs.remove(fut_)
                     if fut_.cancelled():
                         self.logger.debug(f"cb cancelled. {event=}.")
                     elif exc := fut_.exception():

tests/test_util.py

@@ -482,20 +482,30 @@ class TestUtil(ElectrumTestCase):
         async def foo():
             await evt.wait()
 
+        # spawn tasks
         fut = asyncio.ensure_future(foo())
         self.assertTrue(fut in util._running_asyncio_tasks)
         fut = asyncio.create_task(foo())
         self.assertTrue(fut in util._running_asyncio_tasks)
         fut = loop.create_task(foo())
         self.assertTrue(fut in util._running_asyncio_tasks)
-        #fut = asyncio.run_coroutine_threadsafe(foobar(), loop=loop)
+        fut = asyncio.run_coroutine_threadsafe(foo(), loop=loop)
+        # run_coroutine_threadsafe will create a different (chained) future in _running_asyncio_tasks
+        # (which btw will only happen a few event loop iterations later)
         #self.assertTrue(fut in util._running_asyncio_tasks)
 
+        # wait a few event loop iterations
+        for _ in range(10):
+            await asyncio.sleep(0)
         # we should have stored one ref for each above.
         # (though what if test framework is doing stuff ~concurrently?)
-        self.assertEqual(3, len(util._running_asyncio_tasks))
+        self.assertEqual(4, len(util._running_asyncio_tasks))
+        for task in util._running_asyncio_tasks:
+            self.assertEqual(foo().__qualname__, task.get_coro().__qualname__)
+        # let tasks finish
         evt.set()
-        for _ in range(10):  # wait a few event loop iterations
+        # wait a few event loop iterations
+        for _ in range(10):
             await asyncio.sleep(0)
         # refs should be cleaned up by now:
         self.assertEqual(0, len(util._running_asyncio_tasks))