[Dnsmasq-discuss] [PATCH] Do not crash when Ubus connection fails.
etan.kissling at gmail.com
Sun Jun 27 19:19:01 UTC 2021
On 27.06.21, 19:48, "Simon Kelley" <simon at thekelleys.org.uk> wrote:
> My change made the ubus code work in the same way as DBus. It expects
> that ubus_init() will return a non-NULL error report if something
> unexpected and nasty happened. (maybe a configuration that can never
> work.) If the Ubus connection cannot be made, but that's expected to
> change then ubus_init() should return NULL, and leave daemon->ubus set
> to NULL. In that case ubus_init() will be called again, and can either
> succeed, leave daemon->ubus still as NULL (in which case it will be
> called again and again....) or return a fatal error, which can by now
> only be logged. ubus_init() will continue to be called each time through
> the event loop.
Thanks for the really detailed explanations behind the change. It seems
to me that basically, trying to run multiple dnsmasq instances with the
same Ubus is not something that ever was supposed to work. Technically,
as some of the instances quit, a different one could take over the Ubus
connection, but this smells like "doing it incorrectly" in many ways.
I will try to get the embedding project fixed to no longer try register
multiple dnsmasq instances with the same Ubus instance name. This does
not seem to have been right to begin with, entering "undefined behavior"
territory, and now with the change it seems said "undefined behavior"
decided to change. It makes totally sense to me to pick consistency with
DBus over trying to preserve bug compatibility with client projects, so
I no longer see a reason that this patch should be applied to dnsmasq.
> Looking in src/ubus.c, there seems to be a mechanism to reconnect to the
> ubus, and if that fails, daemon->ubus can end up as NULL again, having
> been set up correctly beforehand, so that ubus_init() will start to get
> called again. That code path feels dodgy, and it would be nice to see
> what actually happens when it's run.
> Looking again, there's also a code path in check_ubus_listeners() that
> can delete the ubus connection and set daemon->ubus back to NULL. That
> will start calls to ubus_init() again. I wonder if that is correct?
I'm not familiar enough with that area to comment on this.
More information about the Dnsmasq-discuss