[GCompris-devel] Mutex handling in Download manager

[Holger Kaelberer]

Hi Johnny,

On 27.08.2015 10:26, Johnny Jazeix wrote:
> Hi,
>
> I found a potential deadlock in the DownloadManager.
>
> For some activities, we try to load two rcc in a row: for lang or
> hangman, we need to load words and localized voices.
>
> What is actually done is to load the words.rcc and when loaded, we
> load the voice.
>
> Coding speaking, on onStart activity, we call the registerResource for
> words which locks the rcMutex. Then, as we have connected the
> DownloadManager::resourceRegistered to the
> activity.resourceRegistered, this one calls the Activity.start which
> tries to load the localized voices which will call the
> registerResource and try to lock the (already locked) rcMutex...

Good catch! This can happen if the signal-slot connections are DirectConnection-s, which seems to be 
the case if you experienced a deadlock.

>
> Is there a good practice to avoid that or can we just unlock the mutex
> before emitting the resourceRegister signal?

rcMutex is there to protect registeredResources, therefore it is safe to unlock it right after 
having modified it. Cf. attached patch, if this fixes your issue, pls. commit.

>
> We could also, load the two rcc in the onStart method and waiting for
> both rcc to be loaded to start the activity.

Correct. Having a quick look at the lang branch I found in the kde git (where is hangman, btw?), it 
seems complicated to serialize the download-calls via the resourceRegistered signal. You should be 
able to launch all your downloads via downloadResource() immediately, i.e. in parallel and continue 
processing on resourceRegistered of the last target resource. I had parallel downloads in mind when 
writing the code, although did not test it yet, as we didn't have >1 rcc files in the beginning. 
Could be the right time to give it a try ... ;-)

However, the locking issue must be fixed!

Holger
-------------- next part --------------
A non-text attachment was scrubbed...
Name: core-fix-potential-deadlock-in-DownloadManager.patch
Type: text/x-diff
Size: 587 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/gcompris-devel/attachments/20150827/4bd28b18/attachment.patch>