Re: [Xen-devel] [PATCH v2 3/3] libxl: ocaml: use 'for_app_registration' in osevent callbacks

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Rob Hoes writes ("RE: [PATCH v2 3/3] libxl: ocaml: use 'for_app_registration' 
in osevent callbacks"):
> Ian Jackson wrote:
> > And looking at that, I see that stub_libxl_osevent_occurred_timeout
> > doesn't destroy the for_app.
> 
> Hmm... I thought the for_app stuff is only for the registration
> bits? The osevent_occurred functions don't use or receive it? They
> do get for_libxl, but that's entirely in C and opaque to ocaml.

The usual sequence of events is
  timeout_register
      with your new patch:
           stashes for_libxl value in ocaml gc
           calls ocaml libxl_timeout_register with for_libxl
           stashes that function's return in for_app and adds it to the gc
  timeout occurs
      the timeout machinery calls stub_libxl_osevent_occurred_timeout
          with the for_libxl value it has kept somehow
      stub_libxl_osevent_occurred_timeout calls
          libxl_osevent_occurred_timeout

Now the timeout is gone and nothing will deal with it again.  Who
cleans up the for_app value ?

Perhaps you are confused and don't realise that timeouts are one-shot.
See the comment next to libxl_osevent_occurred_timeout.

> I do assume here that timeout_modify will be called only once for a
> given timeout registration. Is that correct?

The specification is that it may be called more than once, or not at
all.  The cleanup needs to be done in
stub_libxl_osevent_occurred_timeout.

(And you don't probably don't want a binding for timeout_deregister.
That's only there for compatibility with what are now old libxls, and
only if those libxls don't have the race patches which are necessary
for reliable operation.)

Ian.

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