Re: [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed

[Ian Campbell <Ian.Campbell@citrix.com>]

On Fri, 2014-11-28 at 14:56 +0000, Ian Jackson wrote:
> Ian Campbell writes ("Re: [PATCH 2/6] libxl: events: Deregister xenstore watch fd when not needed"):
> > On Thu, 2014-11-27 at 18:27 +0000, Ian Jackson wrote:
> > > * On libxl teardown, the watch fd should therefore be unregistered.
> > >   assert that this is the case.
> > 
> > A bunch of the patches in this series basically assume that the ctx is
> > idle when it is freed, i.e. it requires everything to be explicitly
> > cancelled rather than implicitly doing so on free.
> 
> libxl_ctx_free explicitly disables all the application-requested event
> generators.  (free_disable_deaths and libxl__evdisable_disk_eject.)

So it does, I was looking for that before commenting but didn't see
those calls for what they were, despite the comment right there...

> Destroying the ctx during the execution of an asynchronous operation
> is forbidden by this text in libxl.h (near line 813):
>   * *ao_how does not need to remain valid after the initiating function
>   * returns. All other parameters must remain valid for the lifetime of
>   * the asynchronous operation, unless otherwise specified.
> That implies that the ctx must remain valid during the ao, so it may
> not be destroyed beforehand.
> 
> Those are the two ways that, even without any threads inside libxl, a
> ctx can be other than idle.
> 
> It should be obvious to the application programmer that destroying the
> ctx when there are other threads inside libxl is not going to work.
> Indeed a programmer who tries to destroy the ctx when they have
> threads which might be inside libxl cannot ensure that the ctx is
> valid even on entry to libxl.
> 
> > I think this is a fine restriction, but it probably ought to be written
> > down.
> 
> Does the above demonstrate that the existing restrictions are
> documented ?

Yes.

>   I'd rather avoid writing the restrictions twice if it
> can be avoided - these docs are long enough as they are.

Indeed.