On Fri, 2008-11-14 at 17:09 +0000, Daniel P. Berrange wrote:
On Fri, Nov 14, 2008 at 12:00:10PM -0500, David Lively wrote:
>
> > > +JNIEXPORT void JNICALL Java_org_libvirt_Connect_registerForDomainEvents
> > > +(JNIEnv *env, jobject obj, jlong VCP){
> > > + // TODO: Need to DeleteGlobalRef(obj) when deregistering for
callbacks.
> > > + // But then need to track global obj per Connect object.
> >
> > Hum, that's a bit nasty. Can we make sure we can plug the leaks
> > without having to change the APIs, that would be a bummer...
>
> Yeah. It's really not acceptable as is. The easiest solution (as you
> hint) is changing the API so virConnectDomainEventDeregister returns the
> void * registered with that callback. That would (of course) be my
> preference. What do you think? That API hasn't been released quite
> yet ...
Or have the virConnectDomainEventRegister method take an extra parameter
which is a callback void (*freefunc)(void*). libvirt would just invoke
that to free the opaque data chunk.
Yeah, I like this better. The dbus(?) API allows an optional
destructor (freefunc) to be specified for callback userdata. So let's
allow it to be null (in which case we obviously don't call it at remove
time).
I think we need a similar thing with the event loops APIs for timers
and file handle watches, to make it easier to free the opaque data
blob they have.
Sounds good too.
I can make the DomainEvent changes today / this weekend while working on
the Java bindings (since I need them to plug the Java leak), and submit
them on Monday (or perhaps later today, if I don't get diverted).
Are you going to make the event impl changes?
Thanks,
Dave