On Thu, Jul 30, 2009 at 2:04 AM, Aaron J. Seigo<ase...@kde.org> wrote: > 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: >> 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...
I like the fact that this keeps everything in RunnerManager instead of having the logic separated between files. Doesn't this suffer from the same problem though? If there are no jobs to begin with, the slot won't be called. :( I initially thought the checkTeardown call in RunnerManager::reset would fix this, but reset is called _before_ the dialog is hidden so no signal will be emitted in that particular corner case. I think an easy way around this would be to enqueue a dummy FindMatchesJob (with an invalid context to skip the run method?) when requesting a teardown. I also think RunnerManager::reset should only deal with having a blank search context (but necessarily ending the match session) since the interfaces also call reset when the query changes to an empty string. In the other times RunnerManager::reset() is called, the dialog is about to be closed anyway so matchSessionComplete will be called. > 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. > True. I suppose this would be the best solution since there still is no way to gracefully terminate a thread that's executing a job. >> > 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 :) > Just for clarity I think we should put that in the apidocs. :) - Ryan _______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
