We make a thread-local copy of the SearchContext for each Runner. As such, there is no need to do locking on the thread-local copy. This patch also adds assertions on the non-thread safe methods (e.g. addExactMatch) which are used from thread-local copies, but which should never be used in the shared version.

Also adds a addMatchesTo(SearchContext) which encapsulates this code and makes it safe to delete matches on object deletion, preventing possible memory leaks on SearchContext destruction.

Finally, don't copy all the SearchMatches already registered for each thread-local copy as that's just unused and unecessary overhead.

BIC, though SC.

svn path=/trunk/KDE/kdebase/workspace/libs/plasma/; revision=773473

abstractrunner.cpp

@@ -108,11 +108,7 @@ void AbstractRunner::performMatch( Plasma::SearchContext &globalContext )
     static const int fastEnoughTime = 250;
 
     d->runtime.restart();
-    Plasma::SearchContext localContext( 0, globalContext );
+    SearchContext localContext(0, globalContext);
-    //Keep track of global context list sizes so we know which pointers are our responsibility to delete
-    const int exactEnd = localContext.exactMatches().count();
-    const int possibleEnd = localContext.possibleMatches().count();
-    const int infoEnd = localContext.informationalMatches().count();
 
     match(&localContext);
 
@@ -128,16 +124,9 @@ void AbstractRunner::performMatch( Plasma::SearchContext &globalContext )
         setSpeed(SlowSpeed);
     }
 
-    QList<SearchMatch *> exact = localContext.exactMatches().mid(exactEnd);
-    QList<SearchMatch *> possible = localContext.possibleMatches().mid(possibleEnd);
-    QList<SearchMatch *> info = localContext.informationalMatches().mid(infoEnd);
-
     //If matches were not added, delete items on the heap
-    if (!globalContext.addMatches(localContext.searchTerm(), exact, possible, info)) {
+    if (localContext.addMatchesTo(globalContext) &&
-        qDeleteAll(exact);
+        slowed && runtime < fastEnoughTime) {
-        qDeleteAll(possible);
-        qDeleteAll(info);
-    } else if (slowed && runtime < fastEnoughTime) {
         ++d->fastRuns;
 
         if (d->fastRuns > 2) {

searchcontext.cpp

@@ -39,20 +39,22 @@ namespace Plasma
 class SearchContext::Private
 {
     public:
-        Private()
+        Private(SearchContext::DataPolicy p)
             : type(SearchContext::UnknownType),
-              completer(0)
+              completer(0),
+              policy(p)
         {
         }
 
         ~Private()
         {
+            clearMatches();
             delete completer;
         }
 
         void resetState()
         {
-            lock.lockForWrite();
+            lockForWrite();
             qDeleteAll(info);
             info.clear();
             qDeleteAll(exact);
@@ -66,19 +68,19 @@ class SearchContext::Private
             if (completer) {
                 completer->clear();
             }
-            lock.unlock();
+            unlock();
         }
 
         void clearMatches()
         {
-            lock.lockForWrite();
+            lockForWrite();
             qDeleteAll(info);
             info.clear();
             qDeleteAll(exact);
             exact.clear();
             qDeleteAll(possible);
             possible.clear();
-            lock.unlock();
+            unlock();
         }
 
         KCompletion* completionObject()
@@ -92,18 +94,24 @@ class SearchContext::Private
 
         void lockForRead()
         {
+            if (policy == Shared) {
                 lock.lockForRead();
             }
+        }
 
         void lockForWrite()
         {
+            if (policy == Shared) {
                 lock.lockForWrite();
             }
+        }
 
         void unlock()
         {
+            if (policy == Shared) {
                 lock.unlock();
             }
+        }
 
         QReadWriteLock lock;
         QList<SearchMatch *> info;
@@ -113,23 +121,21 @@ class SearchContext::Private
         QString mimetype;
         SearchContext::Type type;
         KCompletion *completer;
+        const SearchContext::DataPolicy policy;
 };
 
 
-SearchContext::SearchContext(QObject *parent)
+SearchContext::SearchContext(QObject *parent, DataPolicy policy)
     : QObject(parent),
-      d(new Private)
+      d(new Private(policy))
 {
 }
 
 SearchContext::SearchContext(QObject *parent, const SearchContext &other)
-    : QObject( parent ),
+    : QObject(parent),
-      d( new Private )
+      d(new Private(SingleConsumer))
 {
     other.d->lockForRead();
-    d->info = other.d->info;
-    d->possible = other.d->possible;
-    d->exact = other.d->exact;
     d->term = other.d->term;
     d->mimetype = other.d->mimetype;
     d->type = other.d->type;
@@ -251,6 +257,7 @@ void SearchContext::addStringCompletions(const QStringList &completion)
 
 SearchMatch* SearchContext::addInformationalMatch(AbstractRunner *runner)
 {
+    Q_ASSERT(d->policy == SingleConsumer);
     SearchMatch *action = new SearchMatch(this, runner);
     action->setType(SearchMatch::InformationalMatch);
     d->info.append(action);
@@ -259,6 +266,7 @@ SearchMatch* SearchContext::addInformationalMatch(AbstractRunner *runner)
 
 SearchMatch* SearchContext::addExactMatch(AbstractRunner *runner)
 {
+    Q_ASSERT(d->policy == SingleConsumer);
     SearchMatch *action = new SearchMatch(this, runner);
     action->setType(SearchMatch::ExactMatch);
     d->exact.append(action);
@@ -267,6 +275,7 @@ SearchMatch* SearchContext::addExactMatch(AbstractRunner *runner)
 
 SearchMatch* SearchContext::addPossibleMatch(AbstractRunner *runner)
 {
+    Q_ASSERT(d->policy == SingleConsumer);
     SearchMatch *action = new SearchMatch(this, runner);
     action->setType(SearchMatch::PossibleMatch);
     d->possible.append(action);
@@ -296,6 +305,23 @@ bool SearchContext::addMatches( const QString& term, const QList<SearchMatch *>
     return true;
 }
 
+bool SearchContext::addMatchesTo(SearchContext &other)
+{
+    d->lockForRead();
+    if (other.addMatches(d->term, d->exact, d->possible, d->info)) {
+        // the matches no longer belong to this SearchContext,
+        // so remove them from the data
+        d->exact.clear();
+        d->possible.clear();
+        d->info.clear();
+        d->unlock();
+        return true;
+    }
+
+    d->unlock();
+    return false;
+}
+
 QList<SearchMatch *> SearchContext::informationalMatches() const
 {
     d->lockForRead();

searchcontext.h

@@ -52,10 +52,21 @@ class PLASMA_EXPORT SearchContext : public QObject
                     Help
                   };
 
-        explicit SearchContext(QObject *parent = 0);
+        enum DataPolicy { Shared = 0,
+                          SingleConsumer
+                        };
+
+        explicit SearchContext(QObject *parent = 0, DataPolicy policy = Shared);
+
+        /**
+         * Constructs a SearchContext with a DataPolicy of SingleConsumer that
+         * contains the search metadata (though none of the currently registered
+         * matches) from the passed in SearchContext. Primarily useful for creating
+         * a thread-local copy of a Shared SearchContext.
+         */
+        SearchContext(QObject *parent, const SearchContext& other);
         ~SearchContext();
 
-        SearchContext(QObject *parent, const SearchContext& other);
 
         /**
          * Sets the search term for this object and attempts to determine
@@ -112,26 +123,54 @@ class PLASMA_EXPORT SearchContext : public QObject
          *
          * If string data is added to the action using QAction::setData(), that
          * string may be used in user interfaces when the item is selected.
+         *
+         * This may only be used from SingleConsumer SearchContexts, and
+         * does not result in the matchesChanged() signal being emitted.
+         * @see addMatches
          */
         SearchMatch* addInformationalMatch(AbstractRunner *runner);
 
         /**
          * Adds an action that represents an exact match to the current search term.
+         *
+         * This may only be used from SingleConsumer SearchContexts, and
+         * does not result in the matchesChanged() signal being emitted.
+         * @see addMatches
          */
         SearchMatch* addExactMatch(AbstractRunner *runner);
 
         /**
          * Adds an action that represents a possible match to the current search term.
+         *
+         * This may only be used from SingleConsumer SearchContexts, and
+         * does not result in the matchesChanged() signal being emitted.
+         * @see addMatches
          */
         SearchMatch* addPossibleMatch(AbstractRunner *runner);
 
         /**
-         * Appends lists of matches to the lists for exact, possible, and informational matches
+         * Appends lists of matches to the lists for exact, possible, and
-         * @return true if matches were added, false if matches were outdated
+         * informational matches. The SearchContext takes over ownership of the
+         * items on successful addition.
+         *
+         * This method is thread safe and causes the matchesChanged() signal to be emitted.
+         *
+         * @return true if matches were added, false if matches were e.g. outdated
          */
-        bool addMatches( const QString& term, const QList<SearchMatch *> &exactMatches,
+        bool addMatches(const QString& term,
+                        const QList<SearchMatch *> &exactMatches,
                         const QList<SearchMatch *> &possibleMatches,
-                                     const QList<SearchMatch *> &informationalMatches );
+                        const QList<SearchMatch *> &informationalMatches);
+
+        /**
+         * Takes the matches from this SearchContext and adds to them another.
+         * If successful, the matches are removed from this SearchContext and
+         * ownership passed to the other SearchContext
+         *
+         * @arg other the SearchContext to add this object's Matches to
+         * @return true if matches were added, false if matches were e.g. outdated
+         */
+        bool addMatchesTo(SearchContext &other);
 
         /**
          * Retrieves all available informational matches for the current