Review Request: Quit Eventloop before emitting finished and result signals

[Andreas Pakulat]

> On 2009-11-12 14:32:28, Andreas Pakulat wrote:
> > Ok, though I think it would be better if the signal was on the d-pointer instead of the job class itself (as far as I could see that should be possible). KJob already has tons of "this is a private signal don't emit it from a subclass" signals.
> 
> Sebastian Sauer wrote:
>     well, then the private class needs to be changed to inherit from QObject + a Q_OBJECT macro needs to be added. Compared to private slots Qt does not have a Q_PRIVATE_SIGNAL :-/
>

Right,but thats much less a problem (IMHO) than polluting the "public" API with a completely internal signal. I mean as opposed to the others that are declared "private" this one is not even supposed to be used by someone monitoring a job.


- Andreas


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2140/#review3053
-----------------------------------------------------------


On 2009-11-11 22:06:41, Sebastian Sauer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2140/
> -----------------------------------------------------------
> 
> (Updated 2009-11-11 22:06:41)
> 
> 
> Review request for kdelibs.
> 
> 
> Summary
> -------
> 
> If KJob::exec is called then it can happen that the result(KJob*) signal is emitted before the QEventLoop in KJob::exec is quit. If a slot connected with the result(KJob*) signal does then e.g. call the same KJob::exec again then funny things may happen. The patch introduces a new internal signal that is called before the finished and result signals are emitted and that quits the eventloop. This way we can be sure that the finished and result signals are always emitted once the eventloop is done.
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdelibs/kdecore/jobs/kjob.h 1047234 
>   /trunk/KDE/kdelibs/kdecore/jobs/kjob.cpp 1047234 
> 
> Diff: http://reviewboard.kde.org/r/2140/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sebastian
> 
>