kdreview: dictionary runner; changes needed to Plasma::RunnerManager?

[Jason A. Donenfeld]

On Tue, Aug 24, 2010 at 16:59, Aaron J. Seigo <aseigo at kde.org> wrote:
> * when the teardown() signal is emitted, the runner should disconnect from the
> DataEngine

Fixed with r1169251.

> * a thread can wait indefinitely on m_wait.wait(&m_mutex);

But isn't dataUpdated guaranteed to be run at /some point/ ? Or should
we not rely on this.

>
> * if the user types more than $THREAD_POOL_COUNT letters before the dict
> engine returns, the thread pool will be filled with dictionary runner threads
> and therefore be exhausted
>
> looking at it further, it's evident that this runner is really working around
> the fact that it is multithreaded;

Exactly -- the runner is a gigantic hack.


> it would be far easier in this case to
> simply make match() re-entrant and have just one thread of it around.

It'd be nice to be able to access match()'s RunnerContext argument in
a slot, so that match could parse the input, call a method of object
A, and then return, and then a signal coming from A would call a slot
that populates the RunnerContext with matches.

But in such a design what about keeping track of multiple match calls
and multiple RunnerContexts? Always populate the most recent one? a
currentContext() accessor method?

>
> for such runners, i think it would make sense to allow for them to mark
> themselves as re-entrant and then give them their own thread outside the
> shared threadpool.

This would be helpful.


>
> the nice thing about the current design is that match() methods do not need to
> be made re-entrant, and this makes it simpler for many of the runners. but it
> assumes that the runners exit quickly (freeing up the thread pool) and don't
> rely on any resources that have a single-thread restriction on them (as is the
> case with the dictionary plasmoid, among one or two others).

Could we have both designs avaiable controlled by the aforementioned
setReentrant(true) flag?

> once we sort this part out, the i'd like to see the dictonary runner moved to
> kdeplasma-addons/runners/

Hurray!