On Tue, 2020-04-21 at 15:26 +0100, Daniel P. Berrangé wrote:
> 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@redhat.com> 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.
Fair enough. I believe that this cycle issue tends to happen more often
in language bindings. For example, I'm pretty sure that I've seen cases
in the past where reference cycles are introduced between the base
GObject and language binding wrapper objects. So even if the base
library code doesn't have any reference cycles, bindings can introduce
them. But maybe this is not the case for us. I was just being (perhaps
overly) cautious.
Jonathon