bridge: Improve handling of control messages #19637

Remove the general purpose mechanism for sending JSON-formatted messages from the protocol level, leaving only the mechanism for sending control messages. Non-control JSON-formatted messages (such as those used for D-Bus replies) are now handled at the channel level and treated as normal data, subject to flow control. The function for doing so has been renamed from send_message() to send_json() for clarity, and returns boolean (with the same meaning as send_data()), although nobody currently pays any attention to this. This removes one of the main interaction points between endpoints and the router. send_json() uses a class attribute defined on Channel to encode the JSON data. By default, this is the default json.JSONEncoder, but channel subclasses can provide their own encoder (D-Bus needs the encoder provided by systemd_ctypes). This is all driven by a new function in jsonutil which defines the exact way in which we intend to handle keyword args when building control messages. We make a clear distinction between data which is meant to be handled "verbatim" and data which is meant to be subject to processing rules (our '_' to '-' replacements, etc). Start using the new jsonutil function for all control message handling. That means that all control-message-creating paths now offer the opportunity to pass a verbatim dictionary as well as a set of kwargs. Propagate this change upwards, throughout. For forwarding messages from peers this is substantially cleaner because it means we don't rewrite their messages. The improved typing results in some extra mypy errors which requires some hinting at call sites. One note: ideally, we'd use `/` in the argument list of all of these functions to ensure that the "verbatim object" field that we add everywhere can only be passed positionally. This is unfortunately only in Python 3.8. To work around that issue, we add a `_msg` field everywhere with its name chosen never to clash with an actual kwarg we might use (such as `message`). Unfortunately, `mypy` isn't yet convinced that this is a purely positional argument, and in places that we write `**kwargs` it has no way to know that one of the kwargs won't in fact be `_msg`, so it complains that the types don't match. In the cases where that happens, we can manually specify `None` to avoid that problem.

This fixes many issues, including: - Based on AsyncChannel: The new code is based on AsyncChannel and handles blocking operations by using .run_in_executor() instead of creating a working thread. All control flow now happens in a straight line in the run() function. - Implements flow control: The previous code would read many small blocks in a thread and flood the main thread with requests to write them through immediately, without any attempt at flow control. The new code will block if the AsyncChannel send queue backs up. - Better type safety: we use our new JSON helpers to do this. The argument parsing is also less redundant in general since we don't have to do up-front checking and then look everything up again later in the worker thread. - Fewer manual exit paths: use ChannelError instead of manual closes. - Fix header handling: The header handling was incorrect before, compared with the C bridge: we were supposed to always remove 'Connection' and 'Transfer-Encoding', and to remove 'Content-Length' and 'Range' for non-binary channels, but we ended up getting the logic wrong, removing 'Content-Length' and 'Range' for binary channels and 'Conntection' and 'Transfer-Encoding' for text. The behaviour now matches the C bridge. - Safety fixes: The previous code had at least one case of unsafely sending data from the worker thread directly to the channel. This is no longer possible under the new design.

These two exception types are extremely similar and would benefit from being unified. In order to do that, we need to make `Channel.close()` operate on a dictionary (as is found in CockpitProblem.attrs) rather than a set of kwargs (as was previously stored on ChannelError). Add a couple of type signatures around to help us make sure we get this right.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bridge: Improve handling of control messages #19637

bridge: Improve handling of control messages #19637

Commits on Nov 22, 2023