Issues with setupMatchSession/matchSessionComplete in RunnerManager

[Ryan P. Bitanga]

On Thu, Jul 30, 2009 at 9:28 AM, Aaron J. Seigo<aseigo at kde.org> wrote:
> On Wednesday 29 July 2009, Ryan P. Bitanga wrote:
>> On Thu, Jul 30, 2009 at 2:04 AM, Aaron J. Seigo<aseigo at kde.org> wrote:
>> > 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:
>> >>
>> >> 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.
>
> that's why there's a call to checkTearDown() in
> RunnerManager::matchSessionComplete(). if there are no jobs running,
> checkTearDown() will run immediately. otherwise, it will wait until the jobs
> are all done.
>
Oh right... :) In that case apart from not removing the checkTeardown
call in reset I think the patch is good to go.

>> >> 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. :)
>
> sure :)
>
- Ryan