[libvirt] Potential race condition problem

[Benjamin Wang (gendwang)]

Hi,
OK. Now I am using JNA to access libvirt. If we add another mutex which used to access “initialized” parameter. This mutex must be pthread_mutex_init firstly and only once.
But it seems that there is no way to change libvirt code. I do it as following:

1.      Changing libvirt JNA code in Connect.java
Old Code:
    public Connect(String uri) throws LibvirtException {
         VCP = libvirt.virConnectOpen(uri);
        // Check for an error
        processError();
        ErrorHandler.processError(Libvirt.INSTANCE);
    }

New Code:
    public Connect(String uri) throws LibvirtException {
         synchronized(this.getClass()) {
                  VCP = libvirt.virConnectOpen(uri);
         }
        // Check for an error
        processError();
        ErrorHandler.processError(Libvirt.INSTANCE);
    }

This can make sure only that one thread can execute Connect. For a server application, we only need one time. So the performance is OK


2.      Changing libvirt code in libvirt.c
Old Code:
static int initialized = 0;

New Code:
static int volatile initialized = 0;

This can make sure the initialization will be executed once.

Would you give your comments for this solution?

B.R.
Benjamin Wang

From: Guannan Ren [mailto:gren at redhat.com]
Sent: 2012年9月29日 15:43
To: Benjamin Wang (gendwang)
Cc: Daniel Veillard; libvir-list at redhat.com; Yang Zhou (yangzho)
Subject: Re: [libvirt] Potential race condition problem

On 09/29/2012 03:07 PM, Benjamin Wang (gendwang) wrote:
Hi,
   Currently virInitialize() method defined in libvirt.c has the following code:
int
virInitialize(void)
{
    if (initialized)
        return 0;

    initialized = 1;

    if (virThreadInitialize() < 0 ||
        virErrorInitialize() < 0 ||
        virRandomInitialize(time(NULL) ^ getpid()) ||
        virNodeSuspendInit() < 0)
        return -1;

……
}

When two threads access virInitialize method, there is no lock for the “initialized” parameter. If the first thread enters this method and set “initialized” to 1,
the second thread could see that “initialized” is 1(Because initialized is not volatiled, I say could). In some situation, before the first thread finishes all the initialization,
the second thread could use some resources which should be initialized in Initialize method.
If you have any comments, please let me know. Thanks!

B.R.
Benjamin Wang


      As the comments above the function said,
      "It's better to call this routine at startup in multithreaded applications to avoid potential race when initializing the library."


      Guannan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20120929/f3f85ae2/attachment-0001.htm>