[libvirt-users] is libvirt java binding thread safe?

[Matthias Bolte]

2010/9/3 Matthias Bolte <matthias.bolte at googlemail.com>:
> 2010/9/3 Daniel P. Berrange <berrange at redhat.com>:
>> On Fri, Sep 03, 2010 at 01:02:09PM +0200, Matthias Bolte wrote:
>>> Ah, okay, now I understand what you want to say.
>>>
>>> You have two threads A and B. When A runs on its own then it's just
>>> fine, but when you add B (that triggers an exception on purpose and
>>> ignores it) then A picks it up and reports it. This is the point where
>>> thread safety comes to mind.
>>>
>>> libvirt stores errors in thread-local-storage. It uses
>>> pthread_key_create/pthread_{get,set}specific for this (or
>>> TlsAlloc/Tls{Get,Set}Value on Windows).
>>>
>>> I think what's happening here is that all your threads in Java share
>>> the same thread-local-storage. Therefore, thread A can pickup the
>>> error triggered by thread B, that should not happen.
>>>
>>> I'm not sure how to fix that.
>>
>> Looking at the java code, I believe the problem is that the java
>> bindings are *not* using the threadsafe error APIs:
>>
>> In Connect.java
>>
>>    /**
>>     * call the error handling logic. Should be called after every libvirt call
>>     *
>>     * @throws LibvirtException
>>     */
>>    protected void processError() throws LibvirtException {
>>        ErrorHandler.processError(libvirt, VCP);
>>    }
>>
>> Which calls into
>>
>>    public static void processError(Libvirt libvirt, ConnectionPointer conn) throws LibvirtException {
>>        virError vError = new virError();
>>        int errorCode = libvirt.virConnCopyLastError(conn, vError);
>>
>> And virConnCopyLastError is *not* threadsafe:
>>
>>
>> /**
>>  * virConnCopyLastError:
>>  * @conn: pointer to the hypervisor connection
>>  * @to: target to receive the copy
>>  *
>>  * Copy the content of the last error caught on that connection
>>  *
>>  * This method is not protected against access from multiple
>>  * threads. In a multi-threaded application, always use the
>>  * global virGetLastError() API which is backed by thread
>>  * local storage.
>>
>> Regards,
>> Daniel
>
> Hm that might explain it, I'll check if that's the cause.
>
> Matthias
>

Yes, that's the cause. Thanks for the hint :)

I send a patch to the list to fix this.

Matthias