Issues with setupMatchSession/matchSessionComplete in RunnerManager

[Martin Gräßlin]

Am Mittwoch 29 Juli 2009 12:53:56 schrieb Ryan P. Bitanga:
> Hi all,
>
> I should have thought of this sooner, but upon taking a closer look at
> the code, I just realized that we have a few issues with the whole
> setup/teardown scenario.
>
> 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. If a runner deletes something because the
> teardown signal was emitted while a job is still being executed, it's
> possible that the runner may misbehave. In practice that shouldn't be
> much of an issue because we could care less what the old instance
> returns but we need to make this clear in the documentation to avoid
> things like dereferencing null pointers.
>
> Another issue is that teardown is currently only emitted when the
> dialog is hidden. I think the assumption was that there would be a one
> to one correspondence between emissions of prepare and teardown.
> However, with the current implementation, prepare will most likely be
> emitted several more times than teardown. This poses problems for the
> new window runner which only clears the data after the teardown signal
> is emitted. It ends up with too much obsolete data.
As I commented on Aaron's review request: currently teardown signal is never 
fired. And with the prepped bool it should be ensurable that prepare is only 
fired when there has been a teardown before (which is also currently not 
working ;-)). If those two points are fixed there is no problem.
>
> We could leave it up to the runner author to make sure that methods
> connected to the prepare signal clear obsolete data when the query
> changes.
>
> e.g. assuming setupMatch() is connected to prepare and matchComplete()
> is connected to teardown
> void Foo::setupMatch()
> {
>     QMutexLocker l(&m_mutex);
>     // If the query changed before teardown was emitted get rid of the old
> stuff if (data.count()) {
>         matchComplete();
>     }
>     ...
> }
>
> or we could emit teardown prior to launching a new query. Say in
> RunnerManager::setupMatchSession() add
> if (d->prepped) {
>     matchSessionComplete();
> }
I think that is not a good idea. For example the windows runner: it caches all 
icons and window information. That cache should be cleared as soon as the 
query session ended.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 315 bytes
Desc: This is a digitally signed message part.
Url : http://mail.kde.org/pipermail/plasma-devel/attachments/20090729/9f753255/attachment.sig