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 <[email protected]> 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 <[email protected]> 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, [email protected], http://www.davidfaure.fr >> Working on KDE, in particular KDE Frameworks 5 >> >> >
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
