mirror of
https://gitlab.freedesktop.org/NetworkManager/NetworkManager
synced 2024-07-25 04:04:36 +00:00
libnm: queue added/removed signals and suppress uninitialized notifications
Property notifications are queued during object initialization and reloading, but the added/removed signals were emitted immediately even before the object was fully initialized. Additionally, depending on how long asynchronous initialization took, the notifications could have been emitted before the object was fully initialized as deferred_notify_cb() wasn't being suppressed until all the properties were complete. For synchronous intialization, signals could be emitted at various times during initialization and not all of the object's properties may be read. Furthermore property notifications were queued in an idle handler, which breaks users that may not use a mainloop. All signals and notifications should be emitted immediately after initialization is complete for synchronous initialization. To make things consistent and ensure that all signals and notifications are emitted only when initialization is complete, queue signals for deferred emission and only run notifications/signals when all the object's properties have been read. For synchronous initialization, emit all notifications and signals immediately after initialization and not from an idle handler.
This commit is contained in:
Dan Williams
parent
0a0f9a3b4e
commit
52ae28f6e5
|
|
@ -69,7 +69,7 @@ typedef struct {
|
|||
const char *signal_prefix;
|
||||
} PropertyInfo;
|
||||
|
||||
static void reload_complete (NMObject *object);
|
||||
static void reload_complete (NMObject *object, gboolean emit_now);
|
||||
static gboolean demarshal_generic (NMObject *object, GParamSpec *pspec, GVariant *value, gpointer field);
|
||||
|
||||
typedef struct {
|
||||
|
|
@ -83,7 +83,7 @@ typedef struct {
|
|||
NMObject *parent;
|
||||
gboolean suppress_property_updates;
|
||||
|
||||
GSList *notify_props;
|
||||
GSList *notify_items;
|
||||
guint32 notify_id;
|
||||
|
||||
GSList *reload_results;
|
||||
|
|
@ -164,57 +164,180 @@ _nm_object_get_proxy (NMObject *object,
|
|||
return proxy;
|
||||
}
|
||||
|
||||
typedef enum {
|
||||
NOTIFY_SIGNAL_PENDING_NONE,
|
||||
NOTIFY_SIGNAL_PENDING_ADDED,
|
||||
NOTIFY_SIGNAL_PENDING_REMOVED,
|
||||
NOTIFY_SIGNAL_PENDING_ADDED_REMOVED,
|
||||
} NotifySignalPending;
|
||||
|
||||
typedef struct {
|
||||
const char *property;
|
||||
const char *signal_prefix;
|
||||
NotifySignalPending pending;
|
||||
NMObject *changed;
|
||||
} NotifyItem;
|
||||
|
||||
static void
|
||||
notify_item_free (NotifyItem *item)
|
||||
{
|
||||
g_clear_object (&item->changed);
|
||||
g_slice_free (NotifyItem, item);
|
||||
}
|
||||
|
||||
static gboolean
|
||||
deferred_notify_cb (gpointer data)
|
||||
{
|
||||
NMObject *object = NM_OBJECT (data);
|
||||
NMObjectPrivate *priv = NM_OBJECT_GET_PRIVATE (object);
|
||||
NMObjectClass *object_class = NM_OBJECT_GET_CLASS (object);
|
||||
GSList *props, *iter;
|
||||
|
||||
priv->notify_id = 0;
|
||||
|
||||
/* Clear priv->notify_props early so that an NMObject subclass that
|
||||
/* Wait until all reloads are done before notifying */
|
||||
if (priv->reload_remaining)
|
||||
return G_SOURCE_REMOVE;
|
||||
|
||||
/* Clear priv->notify_items early so that an NMObject subclass that
|
||||
* listens to property changes can queue up other property changes
|
||||
* during the g_object_notify() call separately from the property
|
||||
* list we're iterating.
|
||||
*/
|
||||
props = g_slist_reverse (priv->notify_props);
|
||||
priv->notify_props = NULL;
|
||||
props = g_slist_reverse (priv->notify_items);
|
||||
priv->notify_items = NULL;
|
||||
|
||||
g_object_ref (object);
|
||||
|
||||
/* Emit property change notifications first */
|
||||
for (iter = props; iter; iter = g_slist_next (iter)) {
|
||||
g_object_notify (G_OBJECT (object), (const char *) iter->data);
|
||||
g_free (iter->data);
|
||||
NotifyItem *item = iter->data;
|
||||
|
||||
if (item->property)
|
||||
g_object_notify (G_OBJECT (object), item->property);
|
||||
}
|
||||
|
||||
/* And added/removed signals second */
|
||||
for (iter = props; iter; iter = g_slist_next (iter)) {
|
||||
NotifyItem *item = iter->data;
|
||||
char buf[50];
|
||||
gint ret = 0;
|
||||
|
||||
switch (item->pending) {
|
||||
case NOTIFY_SIGNAL_PENDING_ADDED:
|
||||
ret = g_snprintf (buf, sizeof (buf), "%s-added", item->signal_prefix);
|
||||
break;
|
||||
case NOTIFY_SIGNAL_PENDING_REMOVED:
|
||||
ret = g_snprintf (buf, sizeof (buf), "%s-removed", item->signal_prefix);
|
||||
break;
|
||||
case NOTIFY_SIGNAL_PENDING_ADDED_REMOVED:
|
||||
if (object_class->object_creation_failed)
|
||||
object_class->object_creation_failed (object, nm_object_get_path (item->changed));
|
||||
break;
|
||||
case NOTIFY_SIGNAL_PENDING_NONE:
|
||||
default:
|
||||
break;
|
||||
}
|
||||
if (ret > 0) {
|
||||
g_assert (ret < sizeof (buf));
|
||||
g_signal_emit_by_name (object, buf, item->changed);
|
||||
}
|
||||
}
|
||||
g_object_unref (object);
|
||||
|
||||
g_slist_free (props);
|
||||
return FALSE;
|
||||
g_slist_free_full (props, (GDestroyNotify) notify_item_free);
|
||||
return G_SOURCE_REMOVE;
|
||||
}
|
||||
|
||||
static void
|
||||
_nm_object_defer_notify (NMObject *object)
|
||||
{
|
||||
NMObjectPrivate *priv = NM_OBJECT_GET_PRIVATE (object);
|
||||
|
||||
if (!priv->notify_id)
|
||||
priv->notify_id = g_idle_add_full (G_PRIORITY_LOW, deferred_notify_cb, object, NULL);
|
||||
}
|
||||
|
||||
static void
|
||||
_nm_object_queue_notify_full (NMObject *object,
|
||||
const char *property,
|
||||
const char *signal_prefix,
|
||||
gboolean added,
|
||||
NMObject *changed)
|
||||
{
|
||||
NMObjectPrivate *priv;
|
||||
NotifyItem *item;
|
||||
GSList *iter;
|
||||
|
||||
g_return_if_fail (NM_IS_OBJECT (object));
|
||||
g_return_if_fail (!signal_prefix != !property);
|
||||
g_return_if_fail (!signal_prefix == !changed);
|
||||
|
||||
priv = NM_OBJECT_GET_PRIVATE (object);
|
||||
_nm_object_defer_notify (object);
|
||||
|
||||
property = g_intern_string (property);
|
||||
signal_prefix = g_intern_string (signal_prefix);
|
||||
for (iter = priv->notify_items; iter; iter = g_slist_next (iter)) {
|
||||
item = iter->data;
|
||||
|
||||
if (property && (property == item->property))
|
||||
return;
|
||||
|
||||
/* Collapse signals for the same object (such as "added->removed") to
|
||||
* ensure we don't emit signals when their sum should have no effect.
|
||||
* The "added->removed->removed" sequence requires special handling,
|
||||
* hence the addition of the ADDED_REMOVED state to ensure that no
|
||||
* signal is emitted in this case:
|
||||
*
|
||||
* Without the ADDED_REMOVED state:
|
||||
* NONE + added -> ADDED
|
||||
* ADDED + removed -> NONE
|
||||
* NONE + removed -> REMOVED (would emit 'removed' signal)
|
||||
*
|
||||
* With the ADDED_REMOVED state:
|
||||
* NONE | ADDED_REMOVED + added -> ADDED
|
||||
* ADDED + removed -> ADDED_REMOVED
|
||||
* ADDED_REMOVED + removed -> ADDED_REMOVED (emits no signal)
|
||||
*/
|
||||
if (signal_prefix && (changed == item->changed) && (item->signal_prefix == signal_prefix)) {
|
||||
switch (item->pending) {
|
||||
case NOTIFY_SIGNAL_PENDING_ADDED:
|
||||
if (!added)
|
||||
item->pending = NOTIFY_SIGNAL_PENDING_ADDED_REMOVED;
|
||||
break;
|
||||
case NOTIFY_SIGNAL_PENDING_REMOVED:
|
||||
if (added)
|
||||
item->pending = NOTIFY_SIGNAL_PENDING_NONE;
|
||||
break;
|
||||
case NOTIFY_SIGNAL_PENDING_ADDED_REMOVED:
|
||||
if (added)
|
||||
item->pending = NOTIFY_SIGNAL_PENDING_ADDED;
|
||||
break;
|
||||
case NOTIFY_SIGNAL_PENDING_NONE:
|
||||
item->pending = added ? NOTIFY_SIGNAL_PENDING_ADDED : NOTIFY_SIGNAL_PENDING_REMOVED;
|
||||
break;
|
||||
default:
|
||||
g_assert_not_reached ();
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
item = g_slice_new0 (NotifyItem);
|
||||
item->property = property;
|
||||
if (signal_prefix) {
|
||||
item->signal_prefix = signal_prefix;
|
||||
item->pending = added ? NOTIFY_SIGNAL_PENDING_ADDED : NOTIFY_SIGNAL_PENDING_REMOVED;
|
||||
item->changed = changed ? g_object_ref (changed) : NULL;
|
||||
}
|
||||
priv->notify_items = g_slist_prepend (priv->notify_items, item);
|
||||
}
|
||||
|
||||
void
|
||||
_nm_object_queue_notify (NMObject *object, const char *property)
|
||||
{
|
||||
NMObjectPrivate *priv;
|
||||
gboolean found = FALSE;
|
||||
GSList *iter;
|
||||
|
||||
g_return_if_fail (NM_IS_OBJECT (object));
|
||||
g_return_if_fail (property != NULL);
|
||||
|
||||
priv = NM_OBJECT_GET_PRIVATE (object);
|
||||
if (!priv->notify_id)
|
||||
priv->notify_id = g_idle_add_full (G_PRIORITY_LOW, deferred_notify_cb, object, NULL);
|
||||
|
||||
for (iter = priv->notify_props; iter; iter = g_slist_next (iter)) {
|
||||
if (!strcmp ((char *) iter->data, property)) {
|
||||
found = TRUE;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
if (!found)
|
||||
priv->notify_props = g_slist_prepend (priv->notify_props, g_strdup (property));
|
||||
_nm_object_queue_notify_full (object, property, NULL, FALSE, NULL);
|
||||
}
|
||||
|
||||
void
|
||||
|
|
@ -526,17 +649,12 @@ array_diff (GPtrArray *needles, GPtrArray *haystack, GPtrArray *diff)
|
|||
}
|
||||
|
||||
static void
|
||||
emit_added_removed_signal (NMObject *self,
|
||||
const char *signal_prefix,
|
||||
NMObject *changed,
|
||||
gboolean added)
|
||||
queue_added_removed_signal (NMObject *self,
|
||||
const char *signal_prefix,
|
||||
NMObject *changed,
|
||||
gboolean added)
|
||||
{
|
||||
char buf[50];
|
||||
int ret;
|
||||
|
||||
ret = g_snprintf (buf, sizeof (buf), "%s-%s", signal_prefix, added ? "added" : "removed");
|
||||
g_assert (ret < sizeof (buf));
|
||||
g_signal_emit_by_name (self, buf, changed);
|
||||
_nm_object_queue_notify_full (self, NULL, signal_prefix, added, changed);
|
||||
}
|
||||
|
||||
static void
|
||||
|
|
@ -576,17 +694,17 @@ object_property_complete (ObjectCreatedData *odata)
|
|||
|
||||
/* Emit added & removed */
|
||||
for (i = 0; i < removed->len; i++) {
|
||||
emit_added_removed_signal (self,
|
||||
pi->signal_prefix,
|
||||
g_ptr_array_index (removed, i),
|
||||
FALSE);
|
||||
queue_added_removed_signal (self,
|
||||
pi->signal_prefix,
|
||||
g_ptr_array_index (removed, i),
|
||||
FALSE);
|
||||
}
|
||||
|
||||
for (i = 0; i < added->len; i++) {
|
||||
emit_added_removed_signal (self,
|
||||
pi->signal_prefix,
|
||||
g_ptr_array_index (added, i),
|
||||
TRUE);
|
||||
queue_added_removed_signal (self,
|
||||
pi->signal_prefix,
|
||||
g_ptr_array_index (added, i),
|
||||
TRUE);
|
||||
}
|
||||
|
||||
different = removed->len || added->len;
|
||||
|
|
@ -617,8 +735,8 @@ object_property_complete (ObjectCreatedData *odata)
|
|||
if (different && odata->property_name)
|
||||
_nm_object_queue_notify (self, odata->property_name);
|
||||
|
||||
if (priv->reload_results && --priv->reload_remaining == 0)
|
||||
reload_complete (self);
|
||||
if (--priv->reload_remaining == 0)
|
||||
reload_complete (self, FALSE);
|
||||
|
||||
g_object_unref (self);
|
||||
g_free (odata->objects);
|
||||
|
|
@ -661,8 +779,7 @@ handle_object_property (NMObject *self, const char *property_name, GVariant *val
|
|||
odata->array = FALSE;
|
||||
odata->property_name = property_name;
|
||||
|
||||
if (priv->reload_results)
|
||||
priv->reload_remaining++;
|
||||
priv->reload_remaining++;
|
||||
|
||||
path = g_variant_get_string (value, NULL);
|
||||
|
||||
|
|
@ -709,8 +826,7 @@ handle_object_array_property (NMObject *self, const char *property_name, GVarian
|
|||
odata->array = TRUE;
|
||||
odata->property_name = property_name;
|
||||
|
||||
if (priv->reload_results)
|
||||
priv->reload_remaining++;
|
||||
priv->reload_remaining++;
|
||||
|
||||
if (npaths == 0) {
|
||||
object_property_complete (odata);
|
||||
|
|
@ -1029,6 +1145,8 @@ _nm_object_reload_properties (NMObject *object, GError **error)
|
|||
if (!g_hash_table_size (priv->proxies) || !priv->nm_running)
|
||||
return TRUE;
|
||||
|
||||
priv->reload_remaining++;
|
||||
|
||||
g_hash_table_iter_init (&iter, priv->proxies);
|
||||
while (g_hash_table_iter_next (&iter, (gpointer *) &interface, (gpointer *) &proxy)) {
|
||||
ret = g_dbus_proxy_call_sync (priv->properties_proxy,
|
||||
|
|
@ -1048,6 +1166,9 @@ _nm_object_reload_properties (NMObject *object, GError **error)
|
|||
g_variant_unref (ret);
|
||||
}
|
||||
|
||||
if (--priv->reload_remaining == 0)
|
||||
reload_complete (object, TRUE);
|
||||
|
||||
return TRUE;
|
||||
}
|
||||
|
||||
|
|
@ -1131,13 +1252,22 @@ _nm_object_set_property (NMObject *object,
|
|||
}
|
||||
|
||||
static void
|
||||
reload_complete (NMObject *object)
|
||||
reload_complete (NMObject *object, gboolean emit_now)
|
||||
{
|
||||
NMObjectPrivate *priv = NM_OBJECT_GET_PRIVATE (object);
|
||||
GSimpleAsyncResult *simple;
|
||||
GSList *results, *iter;
|
||||
GError *error;
|
||||
|
||||
if (emit_now) {
|
||||
if (priv->notify_id) {
|
||||
g_source_remove (priv->notify_id);
|
||||
priv->notify_id = 0;
|
||||
}
|
||||
deferred_notify_cb (object);
|
||||
} else
|
||||
_nm_object_defer_notify (object);
|
||||
|
||||
results = priv->reload_results;
|
||||
priv->reload_results = NULL;
|
||||
error = priv->reload_error;
|
||||
|
|
@ -1183,7 +1313,7 @@ reload_got_properties (GObject *proxy,
|
|||
}
|
||||
|
||||
if (--priv->reload_remaining == 0)
|
||||
reload_complete (object);
|
||||
reload_complete (object, FALSE);
|
||||
}
|
||||
|
||||
void
|
||||
|
|
@ -1546,8 +1676,8 @@ dispose (GObject *object)
|
|||
priv->notify_id = 0;
|
||||
}
|
||||
|
||||
g_slist_free_full (priv->notify_props, g_free);
|
||||
priv->notify_props = NULL;
|
||||
g_slist_free_full (priv->notify_items, (GDestroyNotify) notify_item_free);
|
||||
priv->notify_items = NULL;
|
||||
|
||||
g_clear_pointer (&priv->proxies, g_hash_table_unref);
|
||||
g_clear_object (&priv->properties_proxy);
|
||||
|
|
|
|||
|
|
@ -111,6 +111,104 @@ test_device_added (void)
|
|||
|
||||
/*******************************************************************/
|
||||
|
||||
typedef enum {
|
||||
SIGNAL_FIRST = 0x01,
|
||||
SIGNAL_SECOND = 0x02,
|
||||
SIGNAL_MASK = 0x0F,
|
||||
NOTIFY_FIRST = 0x10,
|
||||
NOTIFY_SECOND = 0x20,
|
||||
NOTIFY_MASK = 0xF0
|
||||
} DeviceSignaledAfterInitType;
|
||||
|
||||
static void
|
||||
device_sai_added_cb (NMClient *c,
|
||||
NMDevice *device,
|
||||
gpointer user_data)
|
||||
{
|
||||
guint *result = user_data;
|
||||
|
||||
g_assert (device);
|
||||
g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0");
|
||||
|
||||
g_assert ((*result & SIGNAL_MASK) == 0);
|
||||
*result |= *result ? SIGNAL_SECOND : SIGNAL_FIRST;
|
||||
}
|
||||
|
||||
static void
|
||||
devices_sai_notify_cb (NMClient *c,
|
||||
GParamSpec *pspec,
|
||||
gpointer user_data)
|
||||
{
|
||||
guint *result = user_data;
|
||||
const GPtrArray *devices;
|
||||
NMDevice *device;
|
||||
|
||||
devices = nm_client_get_devices (c);
|
||||
g_assert (devices);
|
||||
g_assert_cmpint (devices->len, ==, 1);
|
||||
|
||||
device = g_ptr_array_index (devices, 0);
|
||||
g_assert (device);
|
||||
g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0");
|
||||
|
||||
g_assert ((*result & NOTIFY_MASK) == 0);
|
||||
*result |= *result ? NOTIFY_SECOND : NOTIFY_FIRST;
|
||||
}
|
||||
|
||||
static void
|
||||
test_device_added_signal_after_init (void)
|
||||
{
|
||||
NMClient *client;
|
||||
const GPtrArray *devices;
|
||||
NMDevice *device;
|
||||
guint result = 0;
|
||||
GError *error = NULL;
|
||||
|
||||
sinfo = nm_test_service_init ();
|
||||
client = nm_client_new (NULL, &error);
|
||||
g_assert_no_error (error);
|
||||
|
||||
devices = nm_client_get_devices (client);
|
||||
g_assert (devices->len == 0);
|
||||
|
||||
g_signal_connect (client,
|
||||
NM_CLIENT_DEVICE_ADDED,
|
||||
(GCallback) device_sai_added_cb,
|
||||
&result);
|
||||
|
||||
g_signal_connect (client,
|
||||
"notify::" NM_CLIENT_DEVICES,
|
||||
(GCallback) devices_sai_notify_cb,
|
||||
&result);
|
||||
|
||||
/* Tell the test service to add a new device */
|
||||
nm_test_service_add_device (sinfo, client, "AddWiredDevice", "eth0");
|
||||
|
||||
/* Ensure the 'device-added' signal doesn't show up before
|
||||
* the 'Devices' property change notification */
|
||||
while (!(result & SIGNAL_MASK) && !(result & NOTIFY_MASK))
|
||||
g_main_context_iteration (NULL, TRUE);
|
||||
|
||||
g_signal_handlers_disconnect_by_func (client, device_sai_added_cb, &result);
|
||||
g_signal_handlers_disconnect_by_func (client, devices_sai_notify_cb, &result);
|
||||
|
||||
g_assert ((result & NOTIFY_MASK) == NOTIFY_FIRST);
|
||||
g_assert ((result & SIGNAL_MASK) == SIGNAL_SECOND);
|
||||
|
||||
devices = nm_client_get_devices (client);
|
||||
g_assert (devices);
|
||||
g_assert_cmpint (devices->len, ==, 1);
|
||||
|
||||
device = g_ptr_array_index (devices, 0);
|
||||
g_assert (device);
|
||||
g_assert_cmpstr (nm_device_get_iface (device), ==, "eth0");
|
||||
|
||||
g_object_unref (client);
|
||||
g_clear_pointer (&sinfo, nm_test_service_cleanup);
|
||||
}
|
||||
|
||||
/*******************************************************************/
|
||||
|
||||
static const char *expected_bssid = "66:55:44:33:22:11";
|
||||
|
||||
typedef struct {
|
||||
|
|
@ -1077,6 +1175,7 @@ main (int argc, char **argv)
|
|||
loop = g_main_loop_new (NULL, FALSE);
|
||||
|
||||
g_test_add_func ("/libnm/device-added", test_device_added);
|
||||
g_test_add_func ("/libnm/device-added-signal-after-init", test_device_added_signal_after_init);
|
||||
g_test_add_func ("/libnm/wifi-ap-added-removed", test_wifi_ap_added_removed);
|
||||
g_test_add_func ("/libnm/wimax-nsp-added-removed", test_wimax_nsp_added_removed);
|
||||
g_test_add_func ("/libnm/devices-array", test_devices_array);
|
||||
|
|
|
|||
Loading…
Reference in a new issue