On Tue, Apr 21, 2020 at 04:12:09PM +0200, Rafael Fonseca wrote: > On Tue, Apr 21, 2020 at 4:03 PM Daniel P. Berrangé <berrange@xxxxxxxxxx> wrote: > > > > On Tue, Apr 21, 2020 at 03:48:41PM +0200, Rafael Fonseca wrote: > > > This patch series convert various simple instances of virObject to a > > > GObject equivalent. > > > > > > virLockableObject and virObjects which are subclassed will be covered > > > in future patchsets. > > > > > > New in v2: > > > - use *Dispose for unreffing objects and *Finalize for freeing data, > > > as suggested in the GLib documentation > > > > Can you point to the docs with the rationale for that. Looking at the > > patches the distinction looks pretty arbitary, creating extra methods > > without an obvious benefit. > > https://developer.gnome.org/gobject/stable/gobject-memory.html#gobject-memory-cycles > > I did the changes as requested here: > https://www.redhat.com/archives/libvir-list/2020-April/msg00383.html Sorry I didn't see that suggestion before, as I don't really agree with it. Reading the GObject docs, I see this: "the destruction process is split in two phases: the first phase, executed in the dispose handler is supposed to release all references to other member objects. The second phase, executed by the finalize handler is supposed to complete the object's destruction process. Object methods should be able to run without program error in-between the two phases." And this: "When dispose ends, the object should not hold any reference to any other member object. The object is also expected to be able to answer client method invocations (with possibly an error code but no memory violation) until finalize is executed." The existing libvirt code is written from the POV that everything is released in one time, in a finalize method. Thus by definition our current code has no cycle problems that would require the split dispose/finalize approach. More importantly though, I very much doubt we are able to to satisfy the requirement for "dispose" wrt arbitrary object methods not having memory violations. I'm sure our code expects the objects to be non-NULL and would thus crash on a NULL pointer if run. Overall I'm not convinced there's any benefit to using the separate dispose method in libvirt. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
