Re: [Patch] Some more leaks and memory corruptions

[Dan Williams <dcbw redhat com>]

On Tue, 2006-11-07 at 16:02 +0200, Tambet Ingo wrote:
> Hey,
> 
> Here's a quite long patch for 0.6 branch. In addition to fixes to simple
> leaks which don't need any explanation, I'll try to comment the bigger
> changes and why I did them.
> 
> NetworkManager.c (nm_data_free):
> 
> I added a comment there why it's necessary, but in short, removing all
> devices most likely triggers a state change, which gets scheduled, but
> never has a chance to run. It wouldn't be a big deal, but the scheduled
> signal emitters have references to devices which prevents them getting
> cleaned properly. Also, since these signals now actually do run, the
> order of clean up needed some changes.
> 
> nm-dbus-nmi.c:
> vpn-manager/nm-dbus-vpn.c
> 
> Remove 'dbus_pending_call_ref (pcall)' calls from dbus callbacks - we
> already own them, it just leaks all the pcalls.
> 
> NetworkManagerPolicy.c:
> nm-device.c
> 
> For all the scheduled functions which pass NMActRequest, increment the
> reference count when scheduling and release the reference in callback.
> This is mostly needed to avoid problems when activation is canceled (or
> fails) at unfortunate moments and the NMActRequest gets freed before the
> callback has a chance to run.
> 
> nm-device-802-11-wireless.c:
> 
> There was some unusual reference counting patterns there: in some cases,
> when a callback is scheduled (g_source_attach), the reference count of
> the source was "balanced" at the callback - which works fine if the
> callback actually has the chance to run. Removing a device frees it's
> GMainContext, leaving all scheduled callbacks (and their user_data!)
> alive. So to fix that, the reference of the source is "given" to
> GMainContext, so when it exits, it can cleanly free the sources. Also,
> instead of freeing callback' user_data in callback, use the
> GDestroyNotify of the source to free them - again, so the the
> GMainContext can clean things up without calling the callback.

Sure; looks good.

Dan

> Tambet
> _______________________________________________
> NetworkManager-list mailing list
> NetworkManager-list gnome org
> http://mail.gnome.org/mailman/listinfo/networkmanager-list