Re: [Xen-devel] [PATCH 15/15] libxl: New event generation API

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 12 Dec 2011, Ian Campbell wrote:
> On Mon, 2011-12-12 at 12:30 +0000, Ian Jackson wrote:
> > Ian Campbell writes ("Re: [Xen-devel] [PATCH 15/15] libxl: New event 
> > generation API"):
> > > On Mon, 2011-12-12 at 11:40 +0000, Ian Jackson wrote:
> > > > Well, we have to have an internal version because we don't want to
> > > > call GC_INIT again, obviously.
> > > 
> > > In this case it would take a ctx not a gc and it's ok and expected for
> > > libxl a function calling another libxl function to end up with another
> > > gc in the inner function (it happens all the time).
> > 
> > This is not allowed in functions which might generate events (ie,
> > which might call libxl__event_occurred), because the application's
> > event callback will be called at an unexpected point in libxl's
> > execution.  This has three problems:
> > 
> >  * There is a reentrancy hazard.  Exactly whether this is a problem in
> >    a specific case is difficult to analyse; hence the recording of
> >    pending callbacks in the gc, so that they can be delayed until the
> >    gc is freed.
> > 
> >  * The application's callbacks will be called with the libxl ctx
> >    locked.  This might result in deadlock.
> > 
> >  * The  application's callbacks  might be  called in  the  wrong order
> >    because the  inner gc (containing  later events) is  unwound before
> >    the outer gc (containing earlier events).
> > 
> > In general this means that functions which lock the ctx must not
> > allocate an inner gc.

this is quite an important limitation and change in behaviour!


> I was a little surprised when I looked when writing my previous reply
> that LIBXL_INITGC doesn't actually take nesting into account. I had
> expected that LIBXL_INITGC would return (or somehow chain) the existing
> GC when libxl is calling itself such that all the freeing work (and now
> callbacks) would only happen when exiting the final frame.
> 
> It's probably not so important for memory allocation cleanup but it
> would be a semantically useful change for the callbacks if we could
> arrange for them to happen only on final exit of the library -- exactly
> because of the difficulty of reasoning about things otherwise. Alas I
> can't think of a good way to do this since the ctx can be shared, thread
> local storage would be one or libxl__gc_owner could return a special
> "nested ctx" instead of the real outer ctx, but, ick.

I think that using TLS to increase/decrease a nesting counter would OK
in this case. We could have a libxl__enter and a libxl__exit function
that take care it this.

Modifying libxl__gc_owner could lead to errors because it doesn't handle
the case in which an externally visible function calls another
externally visible function: libxl__gc_owner wouldn't even be called in
this case.



> If we can't figure a way round this then the restriction about libxl not
> calling into itself via its own public interfaces should be documented
> (apologies if I've simply missed that bit, I did go and look because I
> thought I recalled seeing it, but I didn't find it, I think I was
> thinking about the comment associated with the CTX lock). In particular
> if the restriction is that you cannot call public API functions which
> might generate events from within libxl then those functions need to be
> clearly annotated as potentially generating events.

Right.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel