colin/NetworkManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

manager: refactor lifetime handling for idle callback _platform_link_cb_idle()

Browse Source 
We call _platform_link_cb_idle() on idle, so we must take care of the lifetime
of NMManager.

We don't want to take a reference, so that the manager is not kept alive
by platform events.

Refactor the previous implementation with weak pointers to use a linked list
instead. Let's not have any pending idle actions after the manager instance
is destroyed. Instead, properly track and cancel the events.
This commit is contained in:
Thomas Haller
2017-09-29 15:04:53 +02:00
parent 3b3c5843cd
commit 4db253b059
1 changed files with 29 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
src/nm-manager.c
View File

@@ -137,6 +137,8 @@ typedef struct {
} prop_filter; } prop_filter;
NMRfkillManager *rfkill_mgr; NMRfkillManager *rfkill_mgr;
CList link_cb_lst;
NMCheckpointManager *checkpoint_mgr; NMCheckpointManager *checkpoint_mgr;
NMSettings *settings; NMSettings *settings;
@@ -2438,35 +2440,34 @@ platform_link_added (NMManager *self,
} }
typedef struct { typedef struct {
CList lst;
NMManager *self; NMManager *self;
int ifindex; int ifindex;
guint idle_id;
} PlatformLinkCbData; } PlatformLinkCbData;
static gboolean static gboolean
_platform_link_cb_idle (PlatformLinkCbData *data) _platform_link_cb_idle (PlatformLinkCbData *data)
{ {
int ifindex = data->ifindex;
NMManager *self = data->self; NMManager *self = data->self;
NMManagerPrivate *priv; NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
const NMPlatformLink *l; const NMPlatformLink *l;
if (!self) c_list_unlink (&data->lst);
goto out; g_slice_free (PlatformLinkCbData, data);
priv = NM_MANAGER_GET_PRIVATE (self); l = nm_platform_link_get (priv->platform, ifindex);
g_object_remove_weak_pointer (G_OBJECT (self), (gpointer *) &data->self);
l = nm_platform_link_get (priv->platform, data->ifindex);
if (l) { if (l) {
NMPlatformLink pllink; NMPlatformLink pllink;
pllink = *l; /* make a copy of the link instance */ pllink = *l; /* make a copy of the link instance */
platform_link_added (self, data->ifindex, &pllink, FALSE, NULL); platform_link_added (self, ifindex, &pllink, FALSE, NULL);
} else { } else {
NMDevice *device; NMDevice *device;
GError *error = NULL; GError *error = NULL;
device = nm_manager_get_device_by_ifindex (self, data->ifindex); device = nm_manager_get_device_by_ifindex (self, ifindex);
if (device) { if (device) {
if (nm_device_is_software (device)) { if (nm_device_is_software (device)) {
nm_device_sys_iface_state_set (device, NM_DEVICE_SYS_IFACE_STATE_REMOVED); nm_device_sys_iface_state_set (device, NM_DEVICE_SYS_IFACE_STATE_REMOVED);
@@ -2483,8 +2484,6 @@ _platform_link_cb_idle (PlatformLinkCbData *data)
} }
} }
out:
g_slice_free (PlatformLinkCbData, data);
return G_SOURCE_REMOVE; return G_SOURCE_REMOVE;
} }
@@ -2496,17 +2495,22 @@ platform_link_cb (NMPlatform *platform,
int change_type_i, int change_type_i,
gpointer user_data) gpointer user_data)
{ {
NMManager *self;
NMManagerPrivate *priv;
const NMPlatformSignalChangeType change_type = change_type_i; const NMPlatformSignalChangeType change_type = change_type_i;
PlatformLinkCbData *data; PlatformLinkCbData *data;
switch (change_type) { switch (change_type) {
case NM_PLATFORM_SIGNAL_ADDED: case NM_PLATFORM_SIGNAL_ADDED:
case NM_PLATFORM_SIGNAL_REMOVED: case NM_PLATFORM_SIGNAL_REMOVED:
self = NM_MANAGER (user_data);
priv = NM_MANAGER_GET_PRIVATE (self);
data = g_slice_new (PlatformLinkCbData); data = g_slice_new (PlatformLinkCbData);
data->self = NM_MANAGER (user_data); data->self = self;
data->ifindex = ifindex; data->ifindex = ifindex;
g_object_add_weak_pointer (G_OBJECT (data->self), (gpointer *) &data->self); c_list_link_tail (&priv->link_cb_lst, &data->lst);
g_idle_add ((GSourceFunc) _platform_link_cb_idle, data); data->idle_id = g_idle_add ((GSourceFunc) _platform_link_cb_idle, data);
break; break;
default: default:
break; break;
@@ -6156,6 +6160,8 @@ nm_manager_init (NMManager *self)
guint i; guint i;
GFile *file; GFile *file;
c_list_init (&priv->link_cb_lst);
priv->platform = g_object_ref (NM_PLATFORM_GET); priv->platform = g_object_ref (NM_PLATFORM_GET);
priv->capabilities = g_array_new (FALSE, FALSE, sizeof (guint32)); priv->capabilities = g_array_new (FALSE, FALSE, sizeof (guint32));
@@ -6391,10 +6397,18 @@ dispose (GObject *object)
{ {
NMManager *self = NM_MANAGER (object); NMManager *self = NM_MANAGER (object);
NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self); NMManagerPrivate *priv = NM_MANAGER_GET_PRIVATE (self);
CList *iter, *iter_safe;
g_signal_handlers_disconnect_by_func (priv->platform, g_signal_handlers_disconnect_by_func (priv->platform,
G_CALLBACK (platform_link_cb), G_CALLBACK (platform_link_cb),
self); self);
c_list_for_each_safe (iter, iter_safe, &priv->link_cb_lst) {
PlatformLinkCbData *data = c_list_entry (iter, PlatformLinkCbData, lst);
g_source_remove (data->idle_id);
c_list_unlink (iter);
g_slice_free (PlatformLinkCbData, data);
}
g_slist_free_full (priv->auth_chains, (GDestroyNotify) nm_auth_chain_unref); g_slist_free_full (priv->auth_chains, (GDestroyNotify) nm_auth_chain_unref);
priv->auth_chains = NULL; priv->auth_chains = NULL;