tags 642397 +patch thanks Hi!
The problem, as far as I figured it out, is apparently twofold: 1. At some point xfpm_battery_notify() is called with some XfpmBattery object. If notifications are enabled, it will add a source with idle priority which holds a pointer to this object and has xfpm_battery_notify_idle() as its callback function. 2. After xfpm_battery_notify() returns, the XfpmBattery object is entered into some hash table. 3. Later the source invokes xfpm_battery_notify_idle(). The XfpmBattery object that is passed as an argument is usually valid. But sometimes it has a reference count of 0 and priv == NULL. What happens is that sometimes XfpmBattery object is removed from the hash and freed between 2. and 3. Apparently my hal sends an add and an remove message for the same battery immediately after each other. The attached patch fixes this by calling g_object_ref() on the XfpmBatteryObject in xfpm_battery_notify() before g_idle_add() is called to add the source. The corresponding unref happens unconditionally in xfpm_battery_notify_idle() -- that function returns FALSE, which means the source holding the pointer to the XfpmBattery object is removed after it's callback returns. Please, somebody review this patch, I'm really quite new to glib and gobject -- I may have easily overlooked something, or violated some convention. (Like: is the source's callback always called, or are there cases where the source gets removed without beeing called? That would create a memory leak.) Anyway, I'm going to run the patched xfpm for a while and will report back if notice any further problems. Bye, Jö. -- Jorrit (Jö) Fahlke, Interdisciplinary Center for Scientific Computing, Heidelberg University, Im Neuenheimer Feld 368, D-69120 Heidelberg Tel: +49 6221 54 8890 Fax: +49 6221 54 8884 In the beginning the Universe was created. This has made a lot of people very angry and been widely regarded as a bad move. -- Douglas Adams
diff --git a/src/xfpm-battery.c b/src/xfpm-battery.c index a2315ec..73bf3d1 100644 --- a/src/xfpm-battery.c +++ b/src/xfpm-battery.c @@ -242,18 +242,19 @@ xfpm_battery_notify_idle (gpointer data) battery = XFPM_BATTERY (data); message = xfpm_battery_get_message_from_battery_state (battery->priv->state, battery->priv->adapter_present); - if ( !message ) - return FALSE; - - xfpm_notify_show_notification (battery->priv->notify, - _("Xfce power manager"), - message, - xfpm_tray_icon_get_icon_name (battery->priv->icon), - 8000, - battery->priv->type == HAL_DEVICE_TYPE_PRIMARY ? FALSE : TRUE, - XFPM_NOTIFY_NORMAL, - xfpm_tray_icon_get_tray_icon(battery->priv->icon)); - + if ( message ) + xfpm_notify_show_notification (battery->priv->notify, + _("Xfce power manager"), + message, + xfpm_tray_icon_get_icon_name (battery->priv->icon), + 8000, + battery->priv->type == HAL_DEVICE_TYPE_PRIMARY ? FALSE : TRUE, + XFPM_NOTIFY_NORMAL, + xfpm_tray_icon_get_tray_icon(battery->priv->icon)); + + /* returning FALSE removes the source from the main loop, so unref the + user data first */ + g_object_unref (battery); return FALSE; } @@ -275,6 +276,7 @@ xfpm_battery_notify (XfpmBattery *battery) if ( notify ) { + g_object_ref (battery); g_idle_add ((GSourceFunc) xfpm_battery_notify_idle, battery); } }
signature.asc
Description: Digital signature