DVCS crashes and KJob (bug 172309)

[Andreas Pakulat]

On 11.10.08 17:15:46, Evgeniy Ivanov wrote:
> Hi all,
> I've found the following thing in KJob and I think it is causing most
> part of our DVCS crashes:
> http://api.kde.org/4.x-api/kdelibs-apidocs/kdecore/html/kjob_8cpp-source.html#l00186
> Look, start() is implemented in DVCSjob and it can emit result() (or
> connect KProcess to slots emitting it). But it can emit result() before
> loop will be execed and there will be no signals to stop it (deadlock or
> crash depending on deferred deletion).
> I have committed DVCS tests (vcs/dvcs/tests) which prove something
> really wrong with deferred deletion, see dvcsjobTest.cpp:
> First I have a deadlock in first job->exec(). I debugged and saw DVCSjob
> emitted result() before loop is execed. QProcess::waitForStarted()
> before starting a process fixed it.

Interesting that this doesn't seem to be a problem with the svn plugin. It
also has code that calls emitResult inside start() and AFAIK its also uses
with KJob::exec(). However it could be that svn is just more relaxed in the
case of wrong inputs, for example svn info just checks wether the given
location is valid (i.e. KUrl::isValid) and if its not it might still fail
further down the road...

> Then I added another exec for the same job. Depending on
> "job->setAutoDelete(false)" from the dvcsjobTest.cpp, tests got crash or
> deadlock.
> I think the reason is really in KJob::exec(). IMHO we should connect
> result() to KJob::takeResult(){resultIsGot = true;} and do loop.exec()
> only if resultIsGot == false. #qt confirms it.

I think you're right. In theory the eventloop itself could actually check
that but that would need to wait until Qt4.5 is out. However I don't think
we need a new slot, simply put the flag-setting into emitResult, that one
is always called anyway. So the attached patch should work, can you please
test that?

> But I don't understand how deleteLater kills an application. gdb shows
> we stay in loop.exec() from KJob, so we do not return to the main loop
> to activate deletion, right?

If you want to know the answer to that I suggest to look through Qt's event
loop code. I'm not sure wether the delete-event is scheduled only for the
main event loop, it might be that KJob's event loop also executed all
deferred-delete's.

Andreas

-- 
You will be recognized and honored as a community leader.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: kjob_no_loop_when_finished.diff
Type: text/x-diff
Size: 1320 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20081012/8c144845/attachment.bin>