[tor-commits] [stem/master] Resume Synchronous when async methods are called

[atagar at torproject.org]

commit 75834c06d1d2ade574f11c95f980143408350f11
Author: Damian Johnson <atagar at torproject.org>
Date:   Tue Jul 14 14:12:47 2020 -0700

    Resume Synchronous when async methods are called
    
    When our Synchronous class was stopped all further invocations of an async
    method raised a RuntimeError. For most classes (sockets, threads, etc) this is
    proper, but it made working with these objects within synchronous contexts
    error prone.
    
    For example, our Controller's async connect() method resumes our instance, but
    was uncallable due to this behavior. Stopping should be the last action callers
    take, and failing to so so is inconsequential (it simply orphans a daemon
    thread) so erring toward our object always being callable.
---
 stem/util/__init__.py         | 13 ++++++------
 test/unit/util/synchronous.py | 48 +++++++++++--------------------------------
 2 files changed, 18 insertions(+), 43 deletions(-)

diff --git a/stem/util/__init__.py b/stem/util/__init__.py
index c28b0b27..d780a0de 100644
--- a/stem/util/__init__.py
+++ b/stem/util/__init__.py
@@ -213,8 +213,6 @@ class Synchronous(object):
     if self._no_op:
       self.__ainit__()  # this is already an asyncio context
     else:
-      Synchronous.start(self)
-
       # Run coroutines through our loop. This calls methods by name rather than
       # reference so runtime replacements (like mocks) work.
 
@@ -226,6 +224,7 @@ class Synchronous(object):
         elif inspect.ismethod(func) and inspect.iscoroutinefunction(func):
           setattr(self, name, functools.partial(self._run_async_method, name))
 
+      Synchronous.start(self)
       asyncio.run_coroutine_threadsafe(asyncio.coroutine(self.__ainit__)(), self._loop).result()
 
   def __ainit__(self):
@@ -246,8 +245,8 @@ class Synchronous(object):
     #
     # However, when constructed from an asynchronous context the above will
     # likely hang because our loop is already processing a task (namely,
-    # whatever is constructing us). While we can schedule tasks, we cannot
-    # invoke it during our construction.
+    # whatever is constructing us). While we can schedule a follow-up task, we
+    # cannot invoke it during our construction.
     #
     # Finally, when this method is simple we could directly invoke it...
     #
@@ -290,8 +289,8 @@ class Synchronous(object):
   def stop(self) -> None:
     """
     Terminate resources that permits this from being callable from synchronous
-    contexts. Once called any further synchronous invocations will fail with a
-    **RuntimeError**.
+    contexts. Calling either :func:`~stem.util.Synchronous.start` or any async
+    method will resume us.
     """
 
     with self._loop_lock:
@@ -340,7 +339,7 @@ class Synchronous(object):
 
     with self._loop_lock:
       if self._loop is None:
-        raise RuntimeError('%s has been stopped' % type(self).__name__)
+        Synchronous.start(self)
 
       # convert iterator if indicated by this method's name or type hint
 
diff --git a/test/unit/util/synchronous.py b/test/unit/util/synchronous.py
index bfe6113c..d99da922 100644
--- a/test/unit/util/synchronous.py
+++ b/test/unit/util/synchronous.py
@@ -92,7 +92,7 @@ class TestSynchronous(unittest.TestCase):
 
   def test_stop(self):
     """
-    Synchronous callers should receive a RuntimeError when stopped.
+    Stop and resume our instances.
     """
 
     def sync_test():
@@ -100,11 +100,17 @@ class TestSynchronous(unittest.TestCase):
       self.assertEqual('async call', instance.async_method())
       instance.stop()
 
-      self.assertRaises(RuntimeError, instance.async_method)
-
-      # synchronous methods still work
+      # synchronous methods won't resume us
 
       self.assertEqual('sync call', instance.sync_method())
+      self.assertTrue(instance._loop is None)
+
+      # ... but async methods will
+
+      self.assertEqual('async call', instance.async_method())
+      self.assertTrue(isinstance(instance._loop, asyncio.AbstractEventLoop))
+
+      instance.stop()
 
     async def async_test():
       instance = Demo()
@@ -120,7 +126,7 @@ class TestSynchronous(unittest.TestCase):
 
   def test_stop_from_async(self):
     """
-    Ensure we can start and stop our instance from within an async method
+    Ensure we can restart and stop our instance from within an async method
     without deadlock.
     """
 
@@ -135,37 +141,7 @@ class TestSynchronous(unittest.TestCase):
     instance = AsyncStop()
     instance.restart()
     instance.call_stop()
-    self.assertRaises(RuntimeError, instance.call_stop)
-
-  def test_resuming(self):
-    """
-    Resume a previously stopped instance.
-    """
-
-    def sync_test():
-      instance = Demo()
-      self.assertEqual('async call', instance.async_method())
-      instance.stop()
-
-      self.assertRaises(RuntimeError, instance.async_method)
-
-      instance.start()
-      self.assertEqual('async call', instance.async_method())
-      instance.stop()
-
-    async def async_test():
-      instance = Demo()
-      self.assertEqual('async call', await instance.async_method())
-      instance.stop()
-
-      # start has no affect on async users
-
-      instance.start()
-      self.assertEqual('async call', await instance.async_method())
-      instance.stop()
-
-    sync_test()
-    asyncio.run(async_test())
+    self.assertTrue(instance._loop is None)
 
   def test_iteration(self):
     """