ldo / dbussy

Python binding for D-Bus using asyncio
91 stars 22 forks source link

Can not register coroutines with add_filter in asyncio debug mode #7

Closed samuel-gauthier closed 6 years ago

samuel-gauthier commented 6 years ago

Hi,

Starting a script using dbussy with asyncio debug mode makes asynchronous dbus callbacks to stop being called. I tracked the problem down to when the coroutines are called, and it seems to be supported, but it ends up with a traceback.

I have written this patch on dbussy_example bus_monitor to illustrate the problem I face:

diff --git a/bus_monitor b/bus_monitor
index a086df4..fc9cee8 100755
--- a/bus_monitor
+++ b/bus_monitor
@@ -23,7 +23,7 @@ loop = asyncio.get_event_loop()
 # Mainline
 #-

-def handle_message(conn, message, _) :
+async def handle_message(conn, message, _) :
     sys.stdout.write \
       (
             "%s: %s.%s[%s](%s)\n"

Then, launching bus_monitor with the asyncio debug mode :

# PYTHONASYNCIODEBUG=1 ./bus_monitor system
Traceback (most recent call last):
  File "_ctypes/callbacks.c", line 259, in 'converting callback result'
TypeError: an integer is required (got type CoroWrapper)
Exception ignored in: <function Connection.add_filter.<locals>.wrap_function at 0x7fe25766e268>
<CoroWrapper handle_message() running at ./bus_monitor:26, created at /usr/lib/python3.5/asyncio/coroutines.py:80> was never yielded from
Coroutine object created at (most recent call last):
  File "./bus_monitor", line 55, in <module>
    loop.run_forever()
  File "/usr/lib/python3.5/asyncio/base_events.py", line 345, in run_forever
    self._run_once()
  File "/usr/lib/python3.5/asyncio/base_events.py", line 1304, in _run_once
    handle._run()
  File "/usr/lib/python3.5/asyncio/events.py", line 125, in _run
    self._callback(*self._args)
  File "/usr/local/lib/python3.5/dist-packages/dbussy.py", line 1710, in handle_watch_event
    call_dispatch()
  File "/usr/local/lib/python3.5/dist-packages/dbussy.py", line 1689, in call_dispatch
    status = dispatch()
  File "/usr/local/lib/python3.5/dist-packages/dbussy.py", line 2236, in dispatch
    dbus.dbus_connection_dispatch(self._dbobj)
  File "/usr/local/lib/python3.5/dist-packages/dbussy.py", line 2464, in wrap_function
    result = function(self, Message(dbus.dbus_message_ref(message)), user_data)
  File "/usr/lib/python3.5/asyncio/coroutines.py", line 80, in debug_wrapper
    return CoroWrapper(gen, None)
/usr/local/lib/python3.5/dist-packages/dbussy.py:2236: RuntimeWarning: coroutine 'handle_message' was never awaited
  dbus.dbus_connection_dispatch(self._dbobj)

My understanding of the problem is that the proper way to detect if an object is a coroutine is to use the asyncio function iscoroutine, instead of checking if the object is of CoroutineType.

Something like:

- if isinstance(result, types.CoroutineType) :
+ if asyncio.iscoroutine(result) :

If I replace all the occurences of this isinstance(result, types.CoroutineType) by an asyncio.iscoroutine(result), I don't get the tracebacks anymore.

ldo commented 6 years ago

I tried your change to the bus_monitor script, and it works just fine with Python 3.5.5 on Debian Unstable. asyncio.iscoroutine is documented to return True on either a generator or a coroutine object; async def creates a coroutine, not a generator, so that shouldn’t be the issue here.

Perhaps a bug in earlier versions of 3.5.x?

samuel-gauthier commented 6 years ago

Hi again,

Actually my analysis was wrong, sorry about that. My problem is actually not related to this.

What happens is that PYTHONASYNCIODEBUG=1 changes the type of result (in the code below) from CoroutineType into a CoroWrapper type, whose type does not match CoroutineType. I have changed the issue title and description to make things clearer.

It is the asyncio debug mode that triggers the problem, because the CoroWrapper is not of coroutine type, but is catched properly by asyncio.iscoroutine. The script actually works fine without it.

I think it is worth fixing anyway, this debug mode of asyncio is useful (https://docs.python.org/3/library/asyncio-dev.html#debug-mode-of-asyncio). It seems safer to use asyncio's builtin too.

What do you think?

ldo commented 6 years ago

Yup, I can reproduce the problem. I agree debug mode is worth supporting. Fixed.

samuel-gauthier commented 6 years ago

Thanks!