Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce BusConnection#request_name and Connection#object_server to replace #request_service #135

Merged
merged 28 commits into from
Jun 5, 2023

Conversation

mvidner
Copy link
Owner

@mvidner mvidner commented Jun 4, 2023

Problem

In #69: A connection can have more than one well-known name (the letter-dotted one), ruby-dbus doesn't support it.

#request_service assumes you always have one name for a service. Also, it tries to expose this name before you have a chance to export objects. In the simple DBus::Main loop this will work but in other cases (Agama: with a timed EventMachine loop) there is a race.

Solution

Introduce BusConnection#request_name (can have 0 or more of them) and Connection#object_server (that exists before the name is exposed) to replace #request_service.

Testing

mvidner added 27 commits June 2, 2023 15:20
The Service class was serving client and server roles and its #root
tree had ProxyObject or DBus::Object leaves respectively.
User code should not instantiate it directly so the breakage will
hopefully be small.

The main motivation is fixing #69:
serving more than one well-known service names.

ObjectServer, the service-side successor, no longer has any name
(BusConnection, as yet not existing Connection subclass, will
have 0 or more well-known names in addition to its unique name)
It is returned by Connection#object_server and by the deprecated
Connection#request_service.

ProxyService is returned by Connection#service.

Internal changes:
- Renaming `bus` to `connection` where appropriate, not all connections are to
  a bus (but not aiming to fix peer connections right now).
- Object#@service is deprecated, surviving until I figure out the prefered API
  to manage object trees. Object#connection introduced.
- Connection#@service is gone, use Connection#object_server.
- No longer set our name to emitted signals, the bus will overwrite it to our
  unique name anyway.
Simplifying ObjectServer#unexport:
obj.path is always set in its constructor, we don't need to check it

And removing code duplication in Object.dbus_signal
Move service_newapi.rb to mock-service/spaghetti-monster.rb
It must have been built with

    ./configure --enable-verbose-mode
bus.rb was way too big
NodeTree was formerly but shortly called TreeBase
should help the tiny coverage drop
(some calls were made to the system library instead)
the path will always exist because it is fetched from an exported object

coverage obsession
It is largely academic,
noone uses /org/freedesktop/DBus/Local except dbus-daemon.
The name was wrong, a destination is something else.

coverage obsession
NOTE: BusConnection is WIP, its code still lives in Connection
TODO: yardoc note that a failure can still result in getting the name later
because it will kill by process name and it may kill unrelated ones too

It is in practice always wrapped by with_private_bus which will kill the bus
daemon, causing an EOF at our end and our main loop will quit anyway.
any value of the envvar will enable the debug logging
@coveralls
Copy link

coveralls commented Jun 4, 2023

Coverage Status

coverage: 95.808% (+0.6%) from 95.225% when pulling d36f80f on service-vs-proxy-service into 5cea356 on master.

@mvidner
Copy link
Owner Author

mvidner commented Jun 5, 2023

Known problem (@mchf) in OBS build there is no system bus, therefore

expected DBus::Error with message matching /not allowed to own the service/, got #<Errno::ENOENT: No such file or directory - connect(2) for /var/run/dbus/system_bus_socket>

workaround: replace it by xit to disable that one test

because they do not work when building RPMs in OBS

To be replaced by session bus tests where we tell limited-session-bus.conf to
disallow some names.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants