On Wednesday 29 July 2009, Ryan P. Bitanga wrote: > On Thu, Jul 30, 2009 at 12:43 AM, Aaron J. Seigo<ase...@kde.org> wrote: > > On Wednesday 29 July 2009, Ryan P. Bitanga wrote: > >> Because we can launch a new query while jobs for an existing query are > >> still executing, any slots that are connected to prepare and teardown > >> have to be threadsafe. > > > > setup and teadrown should be called and executed from the main thread. > > queries should not be started until those signals (and slots connected to > > them) have been executed. there should be no threading involved at all at > > this point. > > This means that while a query is executing, we can't emit the signals.
that's correct... > Calling matchSessionComplete in the hide event of KRunnerDialog isn't > safe in this case, true... > so I think the best place to do this would be in > the destructor of FindMatchesJob. We'd check if the queue is empty, > and if it is then we can emit the teardown signal. there'd be some corner cases there, such as if there are no FindMatchesJobs when teardown is requested ... how about in RunnerManagerPrivate::jobDone()? see attached patch... the only downside to this approach in general is that one long-running runner (or never returning, even) could indefinitely delay teardown happening for other runners. it would be possible to do per-runner bookkeeping if this every becomes a real issue. > > there'd be no point to these signals at all then and runners would just > > do all their data gathering in (or just before) match(). that would be > > too slow. the point of these signals is to do the setup and teardown as > > rarely as possible, but also only when the data will be used. > > Agreed, but the use case that came to mind while I was writing this > e-mail is that of the window runner. It caches the data once a query > is launched but the data may become invalid while the dialog is open > (e.g. an application is launched/closed while the dialog is open). But > this is an issue addressable in the runner implementation. yes; in this case the setup/teardown lets the runner know that it should start watching for those changes as well. if the data can't be trusted between runs (meaning that there is no way to detect changes and the data may change) then the runner will have to re-load the data in match no matter what we do :) -- Aaron J. Seigo humru othro a kohnu se GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43 KDE core developer sponsored by Qt Software
Index: runnermanager.cpp
===================================================================
--- runnermanager.cpp (revision 1003540)
+++ runnermanager.cpp (working copy)
@@ -57,7 +57,8 @@
RunnerManagerPrivate(RunnerManager *parent)
: q(parent),
deferredRun(0),
- prepped(false)
+ prepped(false),
+ teardownRequested(false)
{
matchChangeTimer.setSingleShot(true);
delayTimer.setSingleShot(true);
@@ -197,8 +198,28 @@
// ourselves here
emit q->matchesChanged(context.matches());
}
+
+ checkTearDown();
}
+ void checkTearDown()
+ {
+ //kDebug() << prepped << teardownRequested << searchJobs.count() << oldSearchJobs.count();
+
+ if (!prepped || !teardownRequested) {
+ return;
+ }
+
+ if (searchJobs.isEmpty() && oldSearchJobs.isEmpty()) {
+ foreach (AbstractRunner *runner, runners) {
+ emit runner->teardown();
+ }
+
+ prepped = false;
+ teardownRequested = false;
+ }
+ }
+
void unblockJobs()
{
// WORKAROUND: Queue an empty job to force ThreadWeaver to awaken threads
@@ -222,6 +243,7 @@
KConfigGroup config;
bool loadAll : 1;
bool prepped : 1;
+ bool teardownRequested : 1;
};
/*****************************************************
@@ -329,6 +351,8 @@
void RunnerManager::setupMatchSession()
{
+ d->teardownRequested = false;
+
if (d->prepped) {
return;
}
@@ -345,11 +369,9 @@
return;
}
- foreach (AbstractRunner *runner, d->runners) {
- emit runner->teardown();
- }
-
- d->prepped = false;
+ kDebug() << "tearing down";
+ d->teardownRequested = true;
+ d->checkTearDown();
}
void RunnerManager::launchQuery(const QString &term)
@@ -480,6 +502,7 @@
}
d->context.reset();
+ d->checkTearDown();
}
} // Plasma namespace
signature.asc
Description: This is a digitally signed message part.
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
