Strange behavior on ravel connection object deletion

[ysard]

Hi, I meet a strange behavior in a kodi plugin using Ravel to handle Bluetooth devices.

The issues that expose the problem and partially resolve it are here:
LibreELEC/LibreELEC.tv#5645
LibreELEC/service.libreelec.settings#245

Basically the code using Ravel is present here:
https://github.com/LibreELEC/service.libreelec.settings/blob/8b1efa30632dd4b5c0493cfab2cc940a927392de/resources/lib/dbus_utils.py#L119-L122

Abbreviated example (it seems to follow your recommendations?):

class LoopThread(threading.Thread):

    def __init__(self, loop):
        super().__init__()
        self.loop = loop
        self.is_stopped = False

    @log.log_function()
    async def wait(self):
        while not self.is_stopped:
            await asyncio.sleep(1)

    @log.log_function()
    def run(self):
        self.loop.run_until_complete(self.wait())
        self.loop.close()

    @log.log_function()
    def stop(self):
        self.is_stopped = True
        self.join()

LOOP = asyncio.get_event_loop()
BUS = ravel.system_bus()
BUS.attach_asyncio(LOOP)
LOOP_THREAD = LoopThread(LOOP)

Calling LOOP_THREAD.stop() is ok but Kodi crashes just after for some reason.

The dbus connection object obtained through Ravel does not seem to be deleted correctly, even if it is explicitly requested with del.
It ends with the following stacktrace after an explicit del BUS:

ERROR <general>: Exception ignored in:
ERROR <general>: <function Connection.__del__ at 0x7f233eb88488>
ERROR <general>:

ERROR <general>: Traceback (most recent call last):
ERROR <general>:   File "/home/pi/.local/lib/python3.7/site-packages/ravel.py", line 287, in __del__
ERROR <general>:
ERROR <general>: remove_listeners(self._client_dispatch, [])
ERROR <general>:
ERROR <general>:   File "/home/pi/.local/lib/python3.7/site-packages/ravel.py", line 268, in remove_listeners
ERROR <general>:
ERROR <general>: for interface in level.interfaces.values() :
ERROR <general>:
ERROR <general>: AttributeError
ERROR <general>: :
ERROR <general>: '_ClientDispatchNode' object has no attribute 'interfaces'

Only a garbage collection solves the problem on the Kodi side.

I am not the developer of this code but I found the way to prevent crash. It is difficult for me to know if it is on the side of dbussy or on the side of the Python code developed on the project.
On the project side it seems to be difficult to know where is the implementation error regarding the dbussy base code.

I am aware that it will also be difficult for you to audit this code base.
However, maybe you can give a hint on what could go wrong and generate the error in Ravel?

Thanks for reading.

[ldo]

Yeah, I can confirm that the Ravel code is currently a bit mixed up in its signal handling. There are two mechanisms for implementing signal listeners: one as part of registering an interface class (ravel.Connection.register()), the other where you register each signal listener individually (ravel.Connection.listen_signal()). The cleanup code is not handling this distinction properly. Will figure out a fix.

[ysard]

Thank you for your quick response; I will follow this issue.

[ldo]

OK, I have changed the code to correctly scan for signal listeners in both places, and only check for registered interfaces for server-side listeners. See if it works better now.

[ysard]

Hi, thank you this is much better on this side, there is no stacktrace anymore.

However an explicit call to del is still mandatory to solve the original problem (crash on shutdown or infinite waiting on close).
Does this del make sense to you?
If so, the problem is in the kodi implementation.

The current mandatory tear down of the code fragment shown above is therefore (I know that many things can happen between initialization and teardown :s):

LOOP_THREAD.stop()
del BUS

[ldo]

I don’t run Kodi, so I won’t be able to replicate that part of the behaviour. Note that the objects returned by ravel.session_bus() and ravel.system_bus() are supposed to wrap “non-private” or “shared” libdbus connections, which means libdbus doesn’t allow you to close them explicitly. Disposing the Python objects does little more than remove the signal listeners.

Also note that you have to explicitly attach a ravel.Connection object to an asyncio event loop, with bus.attach_asyncio(). You don’t seem to do that anywhere?

And I see you run the event loop on a separate thread. While libdbus was originally meant to support multithreading, nevertheless there are some gotchas and bugs that I think they have basically declared unfixable...

[ysard]

Yes the ravel connection is attached to the asyncio loop:

LOOP = asyncio.get_event_loop()
BUS = ravel.system_bus()
BUS.attach_asyncio(LOOP)
LOOP_THREAD = LoopThread(LOOP)

While libdbus was originally meant to support multithreading, nevertheless there are some gotchas and bugs that I think they have basically declared unfixable...

I'll take your word for it :p
The addon in question is called by a program compiled in C++. I don't know more, but intuitively the memory management is sensitive as well as the thread management.

Especially since I have another example of a module which does not use threading/asyncio and which does not have the mandatory del problem.

Thanks for your help, I think we can close this issue.
Could you synchronize this code with PyPi so that the developers can set a minimal version of dbussy to use?