On Wed, 2016-05-04 at 17:27 +0800, Shih-Yuan Lee (FourDollars) wrote:
> ---
> src/devices/nm-device.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
> index ad6f835..77f8874 100644
> --- a/src/devices/nm-device.c
> +++ b/src/devices/nm-device.c
> @@ -1990,8 +1990,8 @@ realize_start_setup (NMDevice *self, const
> NMPlatformLink *plink)
> }
>
> /* trigger initial ip config change to initialize ip-config
> */
> - priv->queued_ip4_config_id = g_idle_add
> (queued_ip4_config_change, self);
> - priv->queued_ip6_config_id = g_idle_add
> (queued_ip6_config_change, self);
> + priv->queued_ip4_config_id = g_idle_add
> (queued_ip4_config_change, g_object_ref (self));
> + priv->queued_ip6_config_id = g_idle_add
> (queued_ip6_config_change, g_object_ref (self));
>
> nm_device_update_hw_address (self);
> nm_device_update_initial_hw_address (self);
> @@ -6982,6 +6982,7 @@ queued_ip4_config_change_clear (NMDevice *self)
> _LOGD (LOGD_DEVICE, "clearing queued IP4 config
> change");
> g_source_remove (priv->queued_ip4_config_id);
> priv->queued_ip4_config_id = 0;
> + g_object_unref (self);
> }
> }
>
> @@ -6994,6 +6995,7 @@ queued_ip6_config_change_clear (NMDevice *self)
> _LOGD (LOGD_DEVICE, "clearing queued IP6 config
> change");
> g_source_remove (priv->queued_ip6_config_id);
> priv->queued_ip6_config_id = 0;
> + g_object_unref (self);
> }
> }
>
> @@ -8942,7 +8944,11 @@ update_ip4_config (NMDevice *self, gboolean
> initial)
> if (activation_source_is_scheduled (self,
> activate_stage5_ip4_conf
> ig_commit,
> AF_INET)) {
> - priv->queued_ip4_config_id = g_idle_add
> (queued_ip4_config_change, self);
> + if (priv->queued_ip4_config_id) {
> + g_source_remove (priv-
> >queued_ip4_config_id);
> + g_object_unref (self);
> + }
> + priv->queued_ip4_config_id = g_idle_add
> (queued_ip4_config_change, g_object_ref (self));
> _LOGT (LOGD_DEVICE, "IP4 update was postponed");
> return;
> }
> @@ -9031,7 +9037,11 @@ update_ip6_config (NMDevice *self, gboolean
> initial)
> if (activation_source_is_scheduled (self,
> activate_stage5_ip6_conf
> ig_commit,
> AF_INET6)) {
> - priv->queued_ip6_config_id = g_idle_add
> (queued_ip6_config_change, self);
> + if (priv->queued_ip6_config_id) {
> + g_source_remove (priv-
> >queued_ip6_config_id);
> + g_object_unref (self);
> + }
> + priv->queued_ip6_config_id = g_idle_add
> (queued_ip6_config_change, g_object_ref (self));
> _LOGT (LOGD_DEVICE, "IP6 update was postponed");
> return;
> }
> @@ -9109,8 +9119,8 @@ queued_ip4_config_change (gpointer user_data)
> return TRUE;
>
> priv->queued_ip4_config_id = 0;
> - g_object_ref (self);
> update_ip4_config (self, FALSE);
> +
> g_object_unref (self);
>
> set_unmanaged_external_down (self, TRUE);
> @@ -9131,7 +9141,6 @@ queued_ip6_config_change (gpointer user_data)
> return TRUE;
>
> priv->queued_ip6_config_id = 0;
> - g_object_ref (self);
> update_ip6_config (self, FALSE);
>
> if ( nm_platform_link_get (NM_PLATFORM_GET, priv->ifindex)
> @@ -9194,7 +9203,7 @@ device_ipx_changed (NMPlatform *platform,
> case NMP_OBJECT_TYPE_IP4_ADDRESS:
> case NMP_OBJECT_TYPE_IP4_ROUTE:
> if (!priv->queued_ip4_config_id) {
> - priv->queued_ip4_config_id = g_idle_add
> (queued_ip4_config_change, self);
> + priv->queued_ip4_config_id = g_idle_add
> (queued_ip4_config_change, g_object_ref (self));
> _LOGD (LOGD_DEVICE, "queued IP4 config
> change");
> }
> break;
> @@ -9211,7 +9220,7 @@ device_ipx_changed (NMPlatform *platform,
> /* fallthrough */
> case NMP_OBJECT_TYPE_IP6_ROUTE:
> if (!priv->queued_ip6_config_id) {
> - priv->queued_ip6_config_id = g_idle_add
> (queued_ip6_config_change, self);
> + priv->queued_ip6_config_id = g_idle_add
> (queued_ip6_config_change, g_object_ref (self));
> _LOGD (LOGD_DEVICE, "queued IP6 config
> change");
> }
> break;Hi, For any pending queued_ip4_config_change(), we track the gsource ID in queued_ip4_config_id. Thus, the right fix is not to take an additional reference for @self, but ensuring that the queued_ip4_config_id is cleared (g_clear_source) before the device object gets destroyed. From looking at the code, it is however unclear how this crash could possibly happen: dispose() -> _cleanup_generic_pre() -> _cleanup_ip4_pre() -> queued_ip4_config_change_clear(). Maybe the right fix for this is https://cgit.freedesktop.org/NetworkManager/NetworkManager/commit/?id=0175056a6d70bafdaf1042eb8f5e1ef57484a3f2 Thomas
signature.asc
Description: This is a digitally signed message part
_______________________________________________ networkmanager-list mailing list [email protected] https://mail.gnome.org/mailman/listinfo/networkmanager-list
