Remap message serials to monotonically increasing value - #81
Open
checkraisefold wants to merge 1 commit into
Open
Remap message serials to monotonically increasing value#81checkraisefold wants to merge 1 commit into
checkraisefold wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As background: this proxy currently relies upon sending "fake" messages to the bus it is proxying, such that the proxy must act as an independent client to the bus separate from any clients connecting to the proxy itself. This is accomplished without causing serial number collisions with the proxy's client(s) by giving these fake messages a very large serial number, and hoping that no client will ever use them.
Unfortunately, this restriction runs into issues in certain scenarios, particularly where nesting two proxies is concerned. When two proxies are nested, the parent proxy will refuse the messages sent by the child proxy and close the connection. In some sandboxing scenarios, nesting two proxy instances is a necessity, or at least is desirable.
Instead of using serial numbers larger than some arbitrarily large constant number for any fake messages, all messages received from the proxy client(s) are remapped to a monotonically increasing serial number per client. This remapping only takes place in the bus-proxy context, and is invisible to the proxy client. Because the proxy is single threaded, this is a safe operation (whereas clients may be multithreaded, and it may be difficult/impossible to guarantee monotonic serials on their end).
When a message is sent by a client for which a reply is expected, we save the original message serial in a hashtable. For the reply, when it's received, the message's reply serial is remapped using the hashtable to the original serial number sent by the proxy client. This makes sure the proxy client can still match the reply message to the original message it sent.
This has been tested and works, and additionally passes the CI test suite on my machine.
The only issue I can think of is a possible memory leak for misbehaving clients/software when a message expecting a reply is sent, but the reply is never sent. This would lead to the serial mapping in the hashtable getting stuck, and never being removed. I'm not sure if this is actually an issue given my lack of complete familiarity with D-Bus/this project, but I'd appreciate maintainer feedback on this/ideas on a solution if it is an issue.
Credit to the Sailfish OS project; this is a modified version of their patch. https://github.com/sailfishos/xdg-dbus-proxy/blob/upgrade-4.4.0/rpm/0003-Use-hash-table-for-mapping-reply-serials-between-con.patch
Fixes #67