From 8e7432d31b38fecbbed585c2d5ae510d24ff5af7 Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Wed, 9 Aug 2023 00:44:33 -1000 Subject: [PATCH] feat: speed up to processing bluez passive data (#221) --- src/dbus_fast/message.pxd | 2 ++ src/dbus_fast/message.py | 23 ++++++++++++-- src/dbus_fast/message_bus.pxd | 14 ++++++--- src/dbus_fast/message_bus.py | 57 +++++++++++++++++++++++++++++------ src/dbus_fast/service.pxd | 12 +++++--- src/dbus_fast/service.py | 2 +- tests/test_marshaller.py | 1 + 7 files changed, 88 insertions(+), 23 deletions(-) diff --git a/src/dbus_fast/message.pxd b/src/dbus_fast/message.pxd index 051007c6..12aa0a41 100644 --- a/src/dbus_fast/message.pxd +++ b/src/dbus_fast/message.pxd @@ -27,6 +27,8 @@ cdef object LITTLE_ENDIAN cdef object PROTOCOL_VERSION cdef object MESSAGE_FLAG +cdef object MESSAGE_FLAG_NONE +cdef object MESSAGE_TYPE_METHOD_CALL cdef get_signature_tree diff --git a/src/dbus_fast/message.py b/src/dbus_fast/message.py index 12e56de3..ecdb5198 100644 --- a/src/dbus_fast/message.py +++ b/src/dbus_fast/message.py @@ -30,6 +30,9 @@ MESSAGE_FLAG = MessageFlag +MESSAGE_FLAG_NONE = MessageFlag.NONE +MESSAGE_TYPE_METHOD_CALL = MessageType.METHOD_CALL + class Message: """A class for sending and receiving messages through the @@ -101,8 +104,8 @@ def __init__( path: Optional[str] = None, interface: Optional[str] = None, member: Optional[str] = None, - message_type: MessageType = MessageType.METHOD_CALL, - flags: Union[MessageFlag, int] = MessageFlag.NONE, + message_type: MessageType = MESSAGE_TYPE_METHOD_CALL, + flags: Union[MessageFlag, int] = MESSAGE_FLAG_NONE, error_name: Optional[Union[str, ErrorType]] = None, reply_serial: int = 0, sender: Optional[str] = None, @@ -153,6 +156,22 @@ def __init__( if not getattr(self, field): raise InvalidMessageError(f"missing required field: {field}") + def __repr__(self) -> str: + """Return a string representation of this message.""" + return ( + f"" + ) + @staticmethod def new_error( msg: "Message", error_name: Union[str, ErrorType], error_text: str diff --git a/src/dbus_fast/message_bus.pxd b/src/dbus_fast/message_bus.pxd index 30a014d1..b3893e72 100644 --- a/src/dbus_fast/message_bus.pxd +++ b/src/dbus_fast/message_bus.pxd @@ -10,12 +10,14 @@ cdef object MessageFlag cdef object MESSAGE_TYPE_CALL cdef object MESSAGE_TYPE_SIGNAL -cdef object NO_REPLY_EXPECTED_VALUE +cdef object NO_REPLY_EXPECTED +cdef object BLOCK_UNEXPECTED_REPLY cdef object assert_object_path_valid cdef object assert_bus_name_valid cdef _expects_reply(Message msg) + cdef class SendReply: cdef object _bus @@ -28,12 +30,12 @@ cdef class BaseMessageBus: cdef public object _user_disconnect cdef public object _method_return_handlers cdef public object _serial - cdef public object _path_exports + cdef public cython.dict _path_exports cdef public cython.list _user_message_handlers - cdef public object _name_owners + cdef public cython.dict _name_owners cdef public object _bus_address cdef public object _name_owner_match_rule - cdef public object _match_rules + cdef public cython.dict _match_rules cdef public object _high_level_client_initialized cdef public object _ProxyObject cdef public object _machine_id @@ -45,7 +47,9 @@ cdef class BaseMessageBus: cpdef _process_message(self, Message msg) @cython.locals( + methods=cython.list, method=_Method, - interface=ServiceInterface + interface=ServiceInterface, + interfaces=cython.list, ) cdef _find_message_handler(self, Message msg) diff --git a/src/dbus_fast/message_bus.py b/src/dbus_fast/message_bus.py index b71f5b87..f037f535 100644 --- a/src/dbus_fast/message_bus.py +++ b/src/dbus_fast/message_bus.py @@ -27,13 +27,34 @@ MESSAGE_TYPE_CALL = MessageType.METHOD_CALL MESSAGE_TYPE_SIGNAL = MessageType.SIGNAL -NO_REPLY_EXPECTED_VALUE = MessageFlag.NO_REPLY_EXPECTED.value +NO_REPLY_EXPECTED = MessageFlag.NO_REPLY_EXPECTED +_LOGGER = logging.getLogger(__name__) + _Message = Message def _expects_reply(msg: _Message) -> bool: - return not (msg.flags.value & NO_REPLY_EXPECTED_VALUE) + """Whether a message expects a reply.""" + return not (msg.flags & NO_REPLY_EXPECTED) + + +def _block_unexpected_reply(reply: _Message) -> None: + """Block a reply if it's not expected. + + Previously we silently ignored replies that were not expected, but this + lead to implementation errors that were hard to debug. Now we log a + debug message instead. + """ + _LOGGER.debug( + "Blocked attempt to send a reply from handler " + "that received a message with flag " + "MessageFlag.NO_REPLY_EXPECTED: %s", + reply, + ) + + +BLOCK_UNEXPECTED_REPLY = _block_unexpected_reply class SendReply: @@ -50,8 +71,7 @@ def __enter__(self): return self def __call__(self, reply: Message) -> None: - if _expects_reply(self._msg): - self._bus.send(reply) + self._bus.send(reply) def _exit( self, @@ -855,9 +875,19 @@ def _process_message(self, msg: _Message) -> None: if msg.message_type is MESSAGE_TYPE_CALL: if not handled: handler = self._find_message_handler(msg) + if not _expects_reply(msg): + if handler: + handler(msg, BLOCK_UNEXPECTED_REPLY) + else: + _LOGGER.error( + '"%s.%s" with signature "%s" could not be found', + msg.interface, + msg.member, + msg.signature, + ) + return send_reply = SendReply(self, msg) - with send_reply: if handler: handler(msg, send_reply) @@ -892,8 +922,11 @@ def _make_method_handler( def _callback_method_handler( msg: Message, send_reply: Callable[[Message], None] ) -> None: + result = method_fn(interface, *msg_body_to_args(msg)) + if not _expects_reply(msg): + return body, fds = fn_result_to_body( - method_fn(interface, *msg_body_to_args(msg)), + result, signature_tree=out_signature_tree, replace_fds=negotiate_unix_fd, ) @@ -911,8 +944,8 @@ def _callback_method_handler( return _callback_method_handler def _find_message_handler( - self, msg - ) -> Optional[Callable[[Message, Callable], None]]: + self, msg: _Message + ) -> Optional[Callable[[Message, Callable[[Message], None]], None]]: if ( msg.interface == "org.freedesktop.DBus.Introspectable" and msg.member == "Introspect" @@ -937,8 +970,12 @@ def _find_message_handler( msg_path = msg.path if msg_path: - for interface in self._path_exports.get(msg_path, []): - for method in ServiceInterface._get_methods(interface): + interfaces = self._path_exports.get(msg_path) + if not interfaces: + return None + for interface in interfaces: + methods = ServiceInterface._get_methods(interface) + for method in methods: if method.disabled: continue diff --git a/src/dbus_fast/service.pxd b/src/dbus_fast/service.pxd index b15c7bd7..8eff7387 100644 --- a/src/dbus_fast/service.pxd +++ b/src/dbus_fast/service.pxd @@ -2,17 +2,19 @@ import cython +from .signature cimport SignatureTree + cdef class _Method: - cdef public object name + cdef public str name cdef public object fn cdef public object disabled cdef public object introspection - cdef public object in_signature - cdef public object out_signature - cdef public object in_signature_tree - cdef public object out_signature_tree + cdef public str in_signature + cdef public str out_signature + cdef public SignatureTree in_signature_tree + cdef public SignatureTree out_signature_tree cdef class ServiceInterface: diff --git a/src/dbus_fast/service.py b/src/dbus_fast/service.py index 6a0bc7a8..19226a1d 100644 --- a/src/dbus_fast/service.py +++ b/src/dbus_fast/service.py @@ -36,7 +36,7 @@ class _Method: - def __init__(self, fn, name, disabled=False): + def __init__(self, fn, name: str, disabled=False): in_signature = "" out_signature = "" diff --git a/tests/test_marshaller.py b/tests/test_marshaller.py index 71268faf..cf141cb5 100644 --- a/tests/test_marshaller.py +++ b/tests/test_marshaller.py @@ -602,5 +602,6 @@ def test_unmarshall_bluez_passive_message(): unmarshaller = Unmarshaller(stream) unmarshaller.unmarshall() message = unmarshaller.message + assert "/org/bluez/hci0/dev_58_D3_49_E6_02_6E" in str(message) unpacked = unpack_variants(message.body) assert unpacked == ["/org/bluez/hci0/dev_58_D3_49_E6_02_6E"]