[vbox-dev] Fix to #13789

[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/20150212/8cfa4f32/attachment.html>