Issues with setupMatchSession/matchSessionComplete in RunnerManager

[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.

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();
}

Thoughts?

Cheers,
Ryan