bridge: Avoid excessive duplicate queries for the D-Bus cache

Specifically, don't call Introspect for things that we already have a
reply for, and don't retrieve properties more than once per batch.

These changes were prompted by the flood of UnitNew and UnitRemoved
signals that systemd sometimes sends, such as during a reload.  When
such a flood comes in, we call cockpit_dbus_cache_poke on
o.fd.systemd1.Manager for each.  If at that time the cache doesn't
already have introspection data and property values for the manager,
it will call the Introspect and GetAll methods once for each signal.

The Introspect calls are done sequentially, but the GetAll calls are
done in parallel.  This might overload the "pending reply limit" of
the D-Bus daemon, at which point things might start to break.
This commit is contained in:
Marius Vollmer 2019-11-08 16:57:15 +02:00 committed by Martin Pitt
parent b685ca417c
commit 3ff2136de1
1 changed files with 55 additions and 8 deletions

View File

@ -544,15 +544,29 @@ introspect_next (CockpitDBusCache *self)
}
else
{
g_debug ("%s: calling Introspect() on %s", self->logname, id->path);
GDBusInterfaceInfo *iface = NULL;
if (id->interface)
iface = cockpit_dbus_interface_info_lookup (self->introspected, id->interface);
if (iface)
{
g_debug ("%s: not calling Introspect() on %s, already done",
self->logname, id->path);
g_queue_pop_head (self->introspects);
introspect_complete (self, id);
introspect_next (self);
}
else
{
g_debug ("%s: calling Introspect() on %s", self->logname, id->path);
id->introspecting = TRUE;
g_dbus_connection_call (self->connection, self->name, id->path,
"org.freedesktop.DBus.Introspectable", "Introspect",
g_variant_new ("()"), G_VARIANT_TYPE ("(s)"),
G_DBUS_CALL_FLAGS_NONE, -1,
self->cancellable, on_introspect_reply,
g_object_ref (self));
id->introspecting = TRUE;
g_dbus_connection_call (self->connection, self->name, id->path,
"org.freedesktop.DBus.Introspectable", "Introspect",
g_variant_new ("()"), G_VARIANT_TYPE ("(s)"),
G_DBUS_CALL_FLAGS_NONE, -1,
self->cancellable, on_introspect_reply,
g_object_ref (self));
}
}
}
}
@ -727,6 +741,28 @@ emit_change (CockpitDBusCache *self,
}
}
static gboolean
find_change (CockpitDBusCache *self,
const gchar *path,
GDBusInterfaceInfo *iface)
{
GHashTable *interfaces;
GHashTable *properties;
if (self->update == NULL)
return FALSE;
interfaces = g_hash_table_lookup (self->update, path);
if (interfaces == NULL)
return FALSE;
properties = g_hash_table_lookup (interfaces, iface->name);
if (properties == NULL)
return FALSE;
return TRUE;
}
static GHashTable *
ensure_interfaces (CockpitDBusCache *self,
const gchar *path)
@ -1635,6 +1671,17 @@ retrieve_properties (CockpitDBusCache *self,
if (g_strcmp0 (iface->name, "org.freedesktop.DBus.Properties") == 0)
return;
/* Only get properties once per batch.
*/
if (find_change (self, path, iface))
{
g_debug ("%s: skipping GetAll for %s at %s, already in batch", self->logname, iface->name, path);
return;
}
emit_change (self, path, iface, NULL, NULL);
g_debug ("%s: calling GetAll() for %s at %s", self->logname, iface->name, path);
gad = g_slice_new0 (GetAllData);