Re: [Xen-devel] [PATCH 22/28] libxl: ocaml: event management

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

The eventy ones I'll mostly need to defer to IanJ, although I'll try and
speculate.

On Tue, 2013-04-23 at 16:33 +0100, Rob Hoes wrote:
> > > +
> > > +value stub_xl_osevent_register_hooks(value ctx, value user) {
> > > + CAMLparam2(ctx, user);
> > > + libxl_osevent_hooks *hooks;
> > > + hooks = malloc(sizeof(*hooks));
> > > +
> > > + hooks->fd_register = fd_register;
> > > + hooks->fd_modify = fd_modify;
> > > + hooks->fd_deregister = fd_deregister;
> > > + hooks->timeout_register = timeout_register;
> > > + hooks->timeout_modify = timeout_modify;
> > > + hooks->timeout_deregister = timeout_deregister;
> > > +
> > > + libxl_osevent_register_hooks(CTX, hooks, (void *) user);
> > 
> > This user thing will be retained by libxl -- is that safe from an ocaml gc 
> > point
> > of view?
> 
> Good point. The original value may go out of scope in the OCaml
> program and will then be GC'ed. We should copy the value to avoid
> trouble. To do that, though, we need to know the type of the thing,
> which is currently polymorphic. I'll just go ahead and make it a
> string instead, because that seems to be the most useful.

Is there not a way to take a GC reference on a value or to otherwise
make it possible for the GC to know you've kept hold of it?

> Are these hooks and associated data every cleaned up by libxl? Or is
> the assumption that libxl_osevent_register_hooks is just called once
> at the beginning of the program, and everything starts till the end?

This one is more of an IanJ thing but I notice that the comment says you
can call it repeatedly, but it's not clear if passing hooks == NULL is a
valid way to unregister things.

libxl itself doesn't ever clean up the hooks, AFAICT. Specifically
libxl_ctx_free() doesn't free them or anything like that.

[...]
> > > +void event_occurs(void *user, const libxl_event *event) {
> > > + CAMLparam0();
> > > + CAMLlocalN(args, 2);
> > > + value *func = caml_named_value("xl_event_occurs_callback");
> > > +
> > > + args[0] = (value) user;
> > > + args[1] = Val_event((libxl_event *) event);
> > > + //libxl_event_free(CTX, event); // no ctx here!
> > 
> > Is it leaked or do you free it somewhere else? I suppose "func" must do it?
> > (which makes sense actually)
> 
> Hmm... This is awkward. The thing we are giving to "func" is the event
> translated into an Ocaml type, and not the C libxl_event*. And even if
> we give the libxl_event* to "func" as well, it still needs to know the
> ctx in order to free it (which it probably would, but won't make
> things easier to use). Is there no way to ask libxl to which ctx the
> event belongs?

Apparently not. I expect the intention was that the void *user would
contain reference to it.

There's nothing to stop you from wrapping the applications user value in
a stub struct which you pass to libxl on register and then unpack here
though. Likewise the application could bundle the CTX into the ocaml
user value and extract it again to use it.

The bigger issue is the const which actually stops you freeing it,
without a horrible cast.
<1365684384.8036.104.camel@xxxxxxxxxxxxxxxxxxxxxx> has more details on
that one.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel