Re: [libvirt] Making the Thread-safety issues with vbox driver ?

On Wednesday 04 April 2012 17:56:12 Jean-Baptiste Rouault wrote: > On Monday 16 January 2012 11:34:53 Matthias Bolte wrote: > > Okay, without looking deeper into this here are some ideas: > > > > The XPCOM API libvirt uses might not be threadsafe, or needs to be > > initialized for every thread that wants to use it. Currently its only > > initialized for the thread that opens the driver. I know that this is > > the case on Windows were VirtualBox uses MSCOM for its API and you > > need to call CoInitialize on every thread. This is currently not done > > for the MSCOM glue in libvirt, so I know that on Windows the > > VirtualBox driver is not threadsafe currently. Also I didn't look into > > a solution for this yet. Maybe we need a thread local variable that > > holds whether (MS/XP)COM was already initialized for this thread and > > add a check to every driver function to initialize it when needed. > > > > Did you try to open a connection for each thread instead of trying to > > share one? If that works reliable it might indicate that there is an > > VirtualBox API initialization problem. > > I tried today with one connection for each thread and it works. > > I changed the vbox driver so that the pfnComInitialize function is called > only when the first connection is opened : it breaks the test, even with > one connection per thread. > > I guess we'll have to use a thread local variable as you suggested, unless > someone has a better idea to handle this problem. Hi, I looked deeper into these thread-safety issues, once a new connection is opened for each thread, everything works well. However, opening and closing connections isn't thread-safe at all for two reasons : - VirtualBox C bindings initialization and uninitialization functions aren't thread-safe. I talked about it with upstream on IRC and they are probably not going to fix it, but would accept a patch fixing the issue. I'm going to contact upstream again to get some advices so I can write a patch. - In the libvirt vbox driver, for each new connection, modification of the global variable g_pVBoxGlobalData isn't protected (see line 1040 of vbox_tmpl.c). First of all, is it really necessary to replace it on each new connection, or would it be ok to initialize it only when the first connection is opened ? A global mutex is needed in the vbox driver to protect access to g_pVBoxGlobalData, the vboxRegister() function seems to be the best place to initialize such a mutex unless there is another entry point to do this ?