Issues with setupMatchSession/matchSessionComplete in RunnerManager

[Aaron J. Seigo]

On Wednesday 29 July 2009, Ryan P. Bitanga wrote:
> On Thu, Jul 30, 2009 at 12:43 AM, Aaron J. Seigo<aseigo at 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: runnermanager_delayed_teardown.diff
Type: text/x-patch
Size: 1979 bytes
Desc: not available
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090729/2417d1c1/attachment.diff 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090729/2417d1c1/attachment.sig