[Nepomuk] High mutex contention in Nepomuk2::Resource

[Simeon Bird]

Ok! I think that is why bug 305024 happens. We create a ResourceWatcher,
and call *most* of its methods via a QAutoConnection, but sometimes call
addResource and removeResource directly. In any cases where the
moveToThread makes a difference, this will surely cause a crash, right?

I guess there are two possible fixes:
1) Just protect the ResourceWatcher with the rm mutex.
2) Call all methods via a QAutoConnection

I like the first, because I don't really understand what is happening with
the moveToThread stuff, but is there some reason I am missing why the mutex
is not enough and this is really necessary?


On 8 March 2013 21:52, Simeon Bird <bladud at gmail.com> wrote:

> I think the answer to my question is "no" because all the places in
> ResourceManager that access the ResourceWatcher were already protected by
> the mutex. So it wasn't that.
>
> The crash occurs in qt code that looks like:
>
> if(pending){
>   dbus_unref(pending)
> }
> and asserts complaining that pending == NULL, so something odd is
> definitely happening.
>
> I just ran kactivities under helgrind, but all I got so far was lots of
> warnings from deep inside Qt, which I think must be false positives...of
> course, I don't seem to hit the crash either, so maybe that's why.
>
> The crash is connected to the ResourceWatcher used inside
> ResourceManagerPrivate, and I have to say that code makes no sense to me...
>
> It creates a new object, then calls QObject->moveToThread()  on it. Then
> some (but not all) of the methods calls on that object are sent using
> QMetaObject. And I think the variable is better protected by just requiring
> caller take the lock, which it already does most of the time.
>
> Actually, maybe that's the problem...
>
> Incidentally, shouldn't the m_dataMutex also be held for
> Nepomuk2::ResourceData::watchEnabled()?
>
>
>
>
> On 8 March 2013 11:52, David Faure <faure at kde.org> wrote:
>
>> On Friday 08 March 2013 11:10:50 Simeon Bird wrote:
>> > > Note that I detected an error in the use of the "modification mutex"
>> within
>> > > Resource. I had used it only for "writes" to variables, but obviously
>> the
>> > >
>> > >> reads must be mutex-protected too. Now that I understand the C++11
>> memory
>> > >> model, it's a lot clearer :)
>> > >> I'll rename the mutex then, it's not a "modification mutex", it's a
>> mutex
>> > >> for
>> > >> the resource member variables.
>> > >
>> > > Uhm. Okay.
>> >
>> > Does the extra locking fix
>> > https://bugs.kde.org/show_bug.cgi?id=305024 which is some sort of
>> > memory corruption in the client-side resource
>> > watcher? It seems to be threading related, since it is intermittent...
>>
>> I can't say for sure. My fixes are not related to DBus at all.
>>
>> Is ResourceWatcher used from multiple threads at the same time? It's not
>> mutex-protected at all. But it doesn't sound like a class that needs
>> thread-
>> safety to me.
>>
>> I suppose the only way to find out if there are still data races will be
>> to run
>> kmail in helgrind again, as I did some time ago...
>>
>> --
>> David Faure, faure at kde.org, http://www.davidfaure.fr
>> Working on KDE, in particular KDE Frameworks 5
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/nepomuk/attachments/20130308/64390c56/attachment.html>