dbusjs / node-dbus-next

🚌 The next great dbus library for node
https://www.npmjs.com/package/dbus-next
155 stars 52 forks source link

Subscribe to `NameOwnerChanged` by default #96

Closed lesha1201 closed 2 years ago

lesha1201 commented 2 years ago

We have a logic for updating the cached name owners https://github.com/dbusjs/node-dbus-next/blob/master/lib/bus.js#L104-L114

This logic only works, if the client were subscribed to NameOwnerChanged signal otherwise we don't get messages of this type. Because of this, when we aren't subscribed to NameOwnerChanged, if we subscribe to signals of some DBus service and then this DBus service is restarted, we will have stale name owners and sender of receiving signal won't match.

I think we can add match for this signal by default (in bus.js) and move this logic to a signal handler. That way we will always have up-to-date name owners and make it tolerant to restarting of some dbus service that it uses.

I can implement it if the maintainers agree with this change.

acrisci commented 2 years ago

This is supposed to happen, yes. It should only be done when the high level client is in use. See here for how it's done in my other library: https://github.com/altdesktop/python-dbus-next/blob/f97900e8568fef14fc5a16bcbd251f6f2f768bd4/dbus_next/message_bus.py#L958-L981

lesha1201 commented 2 years ago

@acrisci Should we implement the same in this library?

acrisci commented 2 years ago

Yeah, make sure you have a test.