Hello again, In response to my recent email, I made this patch which addresses the crash I was experiencing. It is not very pretty, but it seems that there is little control over the order of iteration over the dictionary returned by D-Bus, so something has to be done to work around that. This was the least invasive way I could find to fix things. It is a little bit brittle because create_network() is called (via match_pending_network()) in a g_hash_table_foreach_remove() loop. Such loops do not allow modification of the underlying hash table (pending_networks), and so there is some deliberate logic in create_network() to make things safe. Any feedback would be welcome. Kind regards, Alvin On 6/12/20 5:27 PM, Alvin Šipraga wrote: > org.freedesktop.DBus.ObjectManager.GetManagedObjects returns a > dictionary of <objpath, ...>, which is iterated over in order to > initialize the different objects exposed by iwd. The order of iteration > with respect to the keys (object path) is not guaranteed. > > Network objects in particular can only be properly initialized if the > corresponding device object has already been created. If not, then a > crash is possible when, for example, the iwd plugin handles changes to > the properties of the network object. In general, the code assumes that > the network object is properly initializated. > > Address this problem by storing iwd_network objects without a > corresponding device object in a temporary pending_networks hash table. > When devices are created, the pending_network hash table is then > traveresed and any networks belonging to it are reinitialized suitably. > --- > plugins/iwd.c | 77 +++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 68 insertions(+), 9 deletions(-) > > diff --git a/plugins/iwd.c b/plugins/iwd.c > index bf6a2c26..21ee7b33 100644 > --- a/plugins/iwd.c > +++ b/plugins/iwd.c > @@ -42,6 +42,7 @@ static GDBusProxy *agent_proxy; > static GHashTable *adapters; > static GHashTable *devices; > static GHashTable *networks; > +static GHashTable *pending_networks; > static GHashTable *known_networks; > static GHashTable *stations; > static GHashTable *access_points; > @@ -758,14 +759,14 @@ static char *create_identifier(const char *path, const char *security) > return identifier; > } > > -static void add_network(const char *path, struct iwd_network *iwdn) > +static bool add_network(const char *path, struct iwd_network *iwdn) > { > struct iwd_device *iwdd; > char *identifier; > > iwdd = g_hash_table_lookup(devices, iwdn->device); > if (!iwdd) > - return; > + return false; > > identifier = create_identifier(path, iwdn->type); > iwdn->network = connman_network_create(identifier, > @@ -782,7 +783,7 @@ static void add_network(const char *path, struct iwd_network *iwdn) > if (connman_device_add_network(iwdd->device, iwdn->network) < 0) { > connman_network_unref(iwdn->network); > iwdn->network = NULL; > - return; > + return false; > } > iwdn->iwdd = iwdd; > > @@ -790,6 +791,8 @@ static void add_network(const char *path, struct iwd_network *iwdn) > connman_network_set_group(iwdn->network, identifier); > > g_free(identifier); > + > + return true; > } > > static void remove_network(struct iwd_network *iwdn) > @@ -1240,6 +1243,8 @@ static void create_adapter(GDBusProxy *proxy) > adapter_property_change, NULL); > } > > +static gboolean match_pending_network(gpointer key, gpointer value, gpointer user_data); > + > static void create_device(GDBusProxy *proxy) > { > const char *path = g_dbus_proxy_get_path(proxy); > @@ -1277,6 +1282,9 @@ static void create_device(GDBusProxy *proxy) > device_property_change, NULL); > > add_device(path, iwdd); > + > + g_hash_table_foreach_remove(pending_networks, match_pending_network, > + iwdd); > } > > static void unregister_agent(); > @@ -1413,7 +1421,7 @@ static void iwd_is_out(DBusConnection *conn, void *user_data) > } > } > > -static void create_network(GDBusProxy *proxy) > +static bool create_network(GDBusProxy *proxy) > { > const char *path = g_dbus_proxy_get_path(proxy); > struct iwd_network *iwdn; > @@ -1422,7 +1430,7 @@ static void create_network(GDBusProxy *proxy) > > if (!iwdn) { > connman_error("Out of memory creating IWD network"); > - return; > + return false; > } > > iwdn->path = g_strdup(path); > @@ -1433,7 +1441,7 @@ static void create_network(GDBusProxy *proxy) > if (!iwdn->proxy) { > connman_error("Cannot create IWD network watcher %s", path); > g_hash_table_remove(networks, path); > - return; > + return false; > } > > iwdn->device = g_strdup(proxy_get_string(proxy, "Device")); > @@ -1446,10 +1454,55 @@ static void create_network(GDBusProxy *proxy) > iwdn->device, iwdn->name, iwdn->type, iwdn->connected, > iwdn->known_network); > > - g_dbus_proxy_set_property_watch(iwdn->proxy, > - network_property_change, NULL); > + /* > + * If the device corresponding to this network has not been added yet, > + * add_network() will fail. In this case, treat the network as as > + * pending, and match it when the device has been added. > + */ > + if (!add_network(path, iwdn)) { > + /* > + * Skip this step if the network is already pending, so that > + * pending_networks does not get modified. > + */ > + if (g_hash_table_contains(pending_networks, iwdn->path)) { > + g_hash_table_remove(networks, path); > + } else { > + g_hash_table_steal(networks, path); > + g_hash_table_replace(pending_networks, iwdn->path, > + iwdn); > + DBG("network '%s' is pending", iwdn->name); > + } > + > + return false; > + } > + > + g_dbus_proxy_set_property_watch(iwdn->proxy, network_property_change, > + NULL); > > - add_network(path, iwdn); > + return true; > +} > + > +static gboolean match_pending_network(gpointer key, gpointer value, gpointer user_data) > +{ > + struct iwd_network *iwdn = value; > + struct iwd_device *iwdd = user_data; > + GDBusProxy *proxy; > + > + DBG("network '%s' on device '%s' match device '%s'", iwdn->name, iwdn->device, > + iwdd->path); > + > + if (strcmp(iwdn->device, iwdd->path)) > + return FALSE; > + > + if (!create_network(iwdn->proxy)) { > + connman_error("failed to match network '%s' with device '%s'", > + iwdn->name, iwdd->path); > + return FALSE; > + } > + > + DBG("linked device '%s' with network '%s'", iwdd->name, iwdn->name); > + > + return TRUE; > } > > struct auto_connect_cb_data { > @@ -1734,6 +1787,9 @@ static int iwd_init(void) > networks = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, > network_free); > > + pending_networks = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, > + network_free); > + > known_networks = g_hash_table_new_full(g_str_hash, g_str_equal, NULL, > known_network_free); > > @@ -1782,6 +1838,9 @@ out: > if (networks) > g_hash_table_destroy(networks); > > + if (pending_networks) > + g_hash_table_destroy(pending_networks); > + > if (known_networks) > g_hash_table_destroy(known_networks); > >