bus1 / dbus-broker

Linux D-Bus Message Broker
https://github.com/bus1/dbus-broker/wiki
Apache License 2.0
667 stars 78 forks source link

launch: delay service-start until matches are installed #308

Closed dvdhrm closed 1 year ago

dvdhrm commented 1 year ago

This series delays the service-start until we correctly installed all matches. With #285 we now wait for LoadUnit() to return before installing our D-Bus matches, so we know the proper service object-paths. However, this change caused a race-condition where the service might spawn and fail faster than we can process the reply to LoadUnit(). This is quite unlikely, given that the LoadUnit() operation is scheduled first. However, with the coalesced message-transmissions of dbus-broker, the different messages we schedule are often transmitted in a single syscall and thus on slower machines this race is actually possible to hit.

Regardless, this needs to be fixed, so we simply delay the start-unit operation until the matches are installed. This series starts with a bunch of cleanups and then moves code around to make the series easier to review.

Additionally, given that we now delay access to service->[argc,argv,unit], this series also properly caches the service-information and ensures we do not access changing data.

Possibly fixes #304.

agners commented 1 year ago

I've tested this on the affected system using the commands mentioned in #304 as well as our actual application. In both cases the race-condition no longer occurred, every call ended with the expected response (Could not activate remote peer: unit failed.).

teg commented 1 year ago

Thanks @agners and @dvdhrm !

teg commented 1 year ago

@dvdhrm merge failed, might need a manual rebase?

dvdhrm commented 1 year ago

(rebased on main)