[Libguestfs] [PATCH] generator: c.ml - wrap non deamon function with recursive mutex

[Daniel P. Berrange]

On Fri, May 16, 2014 at 04:59:54PM +0100, Richard W.M. Jones wrote:
> On Fri, May 16, 2014 at 11:47:17AM -0400, Daniel P. Berrange wrote:
> > On Fri, May 16, 2014 at 04:44:41PM +0100, Richard W.M. Jones wrote:
> > > On Thu, May 15, 2014 at 05:39:08PM +0200, mzatko at redhat.com wrote:
> > > > +
> > > > +  pr "gl_recursive_lock_define_initialized(static, global_lock)\n";
> > > 
> > > static?
> > > 
> > > I suspect this only allows one libguestfs handle per process into the
> > > critical section at once.  I think the lock needs to be per-handle
> > > unless I'm misunderstanding what this is for.
> > 
> > Agreed, any locking should be per-handle, and I wouldn't expect to need
> > to use recursive mutexes either. Internal libguestfs code shouldn't be
> > calling back out to the public libguestfs API surely, so shouldn't need
> > to have re-entrancy
> 
> Actually libguestfs does call itself quite a lot.  Whether it should
> call itself through the public API or not is another matter -- we
> could change that fairly easily.  Is there a problem with recursive
> mutexes?

Not a technical issue, rather its more of a philosophical issue. IME, if
you permit recursive mutexes, it allows people to create a tangled mess
of interdepedant/recursive API calls which can become quite hard to
understand over time. Non-recursive mutexes help to force cleaner code
separation.

Of course if libguestfs already calls itself reentrantly via its public
API entry points, the issue is moot, unless you decide you want to spend
the time to refactor it.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|