On Tue, 2016-10-11 at 16:22 +0200, Martin Kletzander wrote: > On Wed, Sep 28, 2016 at 01:41:35PM -0400, Dawid Zamirski wrote: > > The allocation is not guarded by any lock, so there's still a > race. I > think there should be a global struct that has only some lock in it > and > whatever global data you need, the struct will be initialized on the > first call to any function (check out VIR_ONCE_GLOBAL_INIT) and then > the > connection (or global data or how it's called) would be reference > counted (just like you have). It's just that having the reference > count > in the object you will be reallocating over and over again for each > connection is not really good. > Thanks, I see, I'll address this in v2 > I don't understand how vbox works, but if initializing > g_pVBoxGlobalData > does not make any connection, and ISession does (which would make > sense), we could keep the global data around and add ISession for > each connection. I guess that's something you're talking about > below. Neither I'm super familiar with vbox internals, but your suggestion sounds reasonable, so I'll dive into that code in vbox source to find out. > > > > > * _pfnInitialize sets up the virConnectPtr->privateData (aka > > vboxPrivateData) for each connection by copying references to > > ISession and IVirtualBox from g_pVBoxGlobalData so that the rest > > of > > the driver API can use it without referencing the global. Each > > time > > this happens, a conntionCount is incremented on g_pVBoxGlobalData > > to > > keep track of when it's safe to call pfnComUnitialize. One of the > > reasons for existence of per-connection vboxPrivateData rather > > than > > completely relying on vboxGlobalData, is that more modern VBOX > > APIs > > provide pfnClientInitialize (since 4.2.20 and 4.3.4) and > > pfnClientThreadInitialize (5.0+) that allow to create multiple > > instances of ISession so if the code ever gets ported to support > > those newer APIs it should create much less diff noise as all API > > implementations are already updated to assume per-connection > > ISession/IVirutalBox instances. > > > > * _pfnUnitilalize decrements connectionCount in g_pVBoxGlobalData > > and > > once it is down to 0, it calls pfnComUnitialize and > > g_pVBoxGlobalData if free'd. This ensures that pfnComInitialize > > and > > pfnComUnitialize pair is called only once, even when multiple > > concurrent connections are in use. > > That's not true if there is a connection being made while it is being > free()d or it's being allocated in two threads, etc. Unless I missed > something, that is. Understood, though I figure that your initial suggestion to guard allocation of g_pVBoxGlobalData should take care of this? -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list