Re: [Xen-devel] [PATCH v6 03/11] libxl: ocaml: event management

[Rob Hoes <Rob.Hoes@xxxxxxxxxx>]

Ian Campbell wrote:
> On Mon, 2013-12-09 at 15:17 +0000, Rob Hoes wrote:
> > Having bindings to the low-level functions
> > libxl_osevent_register_hooks and related, allows to run an event loop
> > in OCaml; either one we write ourselves, or one that is available
> elsewhere.
> >
> > The Lwt cooperative threads library (http://ocsigen.org/lwt/), which
> > is quite popular these days, has an event loop that can be easily
> > extended to poll any additional fds that we get from libxl. Lwt
> > provides a "lightweight" threading model, which does not let you run
> > any other (POSIX) threads in your application, and therefore excludes
> > an event loop implemented in the C bindings.
> >
> > Signed-off-by: Rob Hoes <rob.hoes@xxxxxxxxxx>
> > CC: David Scott <dave.scott@xxxxxxxxxxxxx>
> > CC: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > CC: Ian Campbell <ian.campbell@xxxxxxxxxx>
> >
> > ---
> > v6: Register "user" values with the OCaml GC, before handing them over
> > to libxl, and unregister when giving them back to OCaml (the latter
> > applies only to the for_callback values that go into asynchronous
> > libxl calls; the (os)event ones are registered once and kept forever).
> 
> I think this could legitimately be part of the main commit message.
> (Except the actual registrations are all in subsequent patches I think,
> just the cleanup is here)
> 
> It is probably worth a comment somewhere explaining that the for_callback
> must always be non-NULL and that it must be registered as a global root.

Ok, I'll add a comment in the source, on top of the async_callback function. 
The registration part is indeed added in a following patch.

> > +value stub_libxl_osevent_register_hooks(value ctx, value user) {
> > +   CAMLparam2(ctx, user);
> > +   CAMLlocal1(result);
> > +   libxl_osevent_hooks *hooks;
> > +   value *p;
> > +
> > +   hooks = malloc(sizeof(*hooks));
> > +   if (!hooks)
> > +           failwith_xl(ERROR_NOMEM, "cannot allocate osevent 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;
> > +
> > +   p = malloc(sizeof(value));
> > +   if (!p)
> > +           failwith_xl(ERROR_NOMEM, "cannot allocate value");
> > +   *p = user;
> > +   caml_register_global_root(p);
> > +
> > +   libxl_osevent_register_hooks(CTX, hooks, (void *) p);
> > +
> > +   result = caml_alloc(1, Abstract_tag);
> > +   *((libxl_osevent_hooks **) result) = hooks;
> > +
> > +   CAMLreturn(result);
> 
> Why do we need to return this results thing which is a pointer to the
> hooks pointer? Something to do with keeping it live in the gc? Doesn't
> that rely on the caller assigning it to a long lived variable so that it
> isn't gc'd?

I think I made it return a pointer to the hooks, to be able to use it as a 
handle that we can give to a cleanup function. However, no such cleanup 
function currently exists (yet), and we don't actually use the return value.

The pointer is wrapped in an ocaml heap block with an Abstract_tag, which means 
that the GC will ignore whatever is inside. The block itself may be GC'ed if it 
goes out of scope in the ocaml code (which implies the value is not actually 
used in ocaml), but this won't affect the actual struct on the (C) heap.

Cheers,
Rob

> > +value stub_libxl_event_register_callbacks(value ctx, value user) {
> > +   CAMLparam2(ctx, user);
> > +   CAMLlocal1(result);
> > +   struct user_with_ctx *c_user = NULL;
> > +   libxl_event_hooks *hooks;
> > +
> > +   c_user = malloc(sizeof(*c_user));
> > +   if (!c_user)
> > +           failwith_xl(ERROR_NOMEM, "cannot allocate user value");
> > +   c_user->user = user;
> > +   c_user->ctx = CTX;
> > +   caml_register_global_root(&c_user->user);
> > +
> > +   hooks = malloc(sizeof(*hooks));
> > +   if (!hooks)
> > +           failwith_xl(ERROR_NOMEM, "cannot allocate event hooks");
> > +   hooks->event_occurs_mask = LIBXL_EVENTMASK_ALL;
> > +   hooks->event_occurs = event_occurs;
> > +   hooks->disaster = disaster;
> > +
> > +   libxl_event_register_callbacks(CTX, hooks, (void *) c_user);
> > +   result = caml_alloc(1, Abstract_tag);
> > +   *((libxl_event_hooks **) result) = hooks;
> > +
> > +   CAMLreturn(result);
> 
> Same thing again...
> 
> Ian.

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