[vbox-dev] Fix to #13789

Klaus Espenlaub klaus.espenlaub at oracle.com
Tue Feb 17 15:45:38 GMT 2015


Alexander,

all your fixes (including the relevant subset of this one) are included 
in 4.3.22. Would be great if you could provide feedback if any deadlocks 
or other quirks are left.

Regards,
Klaus
On 12.02.2015 12:42, a.urakov at drweb.com wrote:
> Hi Klaus,
>
> you are right, if approved Medium lock order is "from parent to 
> child", then there is nothing wrong in original VirtualBoxImpl.cpp 
> code. Thanks a lot!
>
> Regards,
> Alexander
>
> 12.02.2015 14:22, Klaus Espenlaub пишет:
>> Hi Alexander,
>>
>> this patch (the VirtualBoxImpl.cpp part) is loosening up 
>> synchronization a bit too much for my taste. The approved lock order 
>> for Medium instances is "from parent to child", which is why your 
>> MachineImpl.cpp change is a perfect catch (and solution, because the 
>> child lock is held unnecessarily).
>>
>> The VirtualBoxImpl.cpp part on the other hand uses the correct lock 
>> order, so I'm wondering if you have any evidence that this is 
>> actually solving any deadlock. Not disputing that the code should 
>> still work properly after the change (with a tiny bit less atomicity 
>> which isn't strictly required), but I suspect there was nothing wrong 
>> with it initially. If it played part of a deadlock then it's most 
>> likely the other party where the lock order is wrong. If you're 
>> running a debug build then the lock validator in the runtime is 
>> active and should catch anything fishy (unless it involves event 
>> semapshores as they have no owner).
>>
>> Feedback appreciated :-)
>>
>> Klaus
>>
>>
>> On 30.01.2015 15:50, a.urakov at drweb.com wrote:
>>> Diffs attached
>>>
>>> 30.01.2015 17:49, a.urakov at drweb.com пишет:
>>>> Hello!
>>>>
>>>> We would like to contribute to VirtualBox under MIT license. We 
>>>> send our changes to fix bug described at 
>>>> https://www.virtualbox.org/ticket/13789 .
>>>>
>>>> The idea is that functions /Medium::removeRegistry()/ and 
>>>> /Medium::addRegistry()/ themselves take write locks on mediums, so 
>>>> we get read lock only for /Medium::getAnyMachineBackref()/ call in 
>>>> /VirtualBox::unregisterMachine()/ function. Also we release read 
>>>> lock on medium after we got its parent and before we are going to 
>>>> iterate through parents in /Machine::detachAllMedia()/ function.
>>>>
>>>> Regards,
>>>> Alexander 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20150217/ad5a19d5/attachment.html>


More information about the vbox-dev mailing list