[vbox-dev] [SPAM] Re: Fix to #13789

a.urakov at drweb.com a.urakov at drweb.com
Thu Feb 12 11:42:01 GMT 2015


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
>>>
>
>
> _______________________________________________
> vbox-dev mailing list
> vbox-dev at virtualbox.org
> https://www.virtualbox.org/mailman/listinfo/vbox-dev

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.virtualbox.org/pipermail/vbox-dev/attachments/20150212/0f386b4a/attachment.html>


More information about the vbox-dev mailing list