----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/105562/#review16364 -----------------------------------------------------------
libnepomukcore/resource/resource.cpp <http://git.reviewboard.kde.org/r/105562/#comment12833> So determineUri returns something that can be deleted at any time? That sounds broken (and should be value-based or shared-pointer-based). It means the caller needs to acquire the resourcedata's m_modificationMutex, basically... which isn't practical. Do you mean that the idea was that this was protected by the resourcemanager (rm) mutex? That's a bit weird, that mutex is supposed to be about the resource manager itself (m_rm), which isn't involved in this call AFAICS (except as an implementation detail for the insert(m_uri,this) in the rest of the patch). That's the problem and the reason I moved the rm lock down: it can be needed inside determineUri()... Are you saying that we should instead ask all callers of determineUri() to acquire the rm lock? That's what determineFinalResourceData did, but all other callers don't, currently. - David Faure On July 13, 2012, 9:24 p.m., David Faure wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/105562/ > ----------------------------------------------------------- > > (Updated July 13, 2012, 9:24 p.m.) > > > Review request for Nepomuk, Vishesh Handa and Sebastian Trueg. > > > Description > ------- > > Fix race conditions in nepomuk's ResourceData. > > Found with "helgrind kmail --nofork", inbox, clicking on any email. > where helgrind is "QT_NO_GLIB=1 valgrind --tool=helgrind > --track-lockorders=no" > > The change in resource.cpp avoids a deadlock due to the change > in resourcedata: no need to hold the rmMutex for the determineUri call. > > > Diffs > ----- > > libnepomukcore/resource/resource.cpp > c237f44c1420929143299aab391a0f2a7709f894 > libnepomukcore/resource/resourcedata.cpp > e19b4bdcd3bea11c32673d13df665cff24783e06 > > Diff: http://git.reviewboard.kde.org/r/105562/diff/ > > > Testing > ------- > > > Thanks, > > David Faure > >
_______________________________________________ Nepomuk mailing list [email protected] https://mail.kde.org/mailman/listinfo/nepomuk
