Don't block the main thread while running CliInterface jobs
ClosedPublic

Authored by elvisangelaccio on Apr 29 2016, 4:48 PM.

Details

Summary

Currently we freeze the GUI thread while running CliInterface jobs, because
there is a nested event loop in runProcess(), which doesn't run anymore in a
separate thread.

This is of course bad and for example is currently impossible to close Ark
while loading a huge zip file.

We can simply drop this event loop and rework the CliInterface logic, such that
the finished() signal is not emitted anymore when runProcess() returns, but
in processFinished() instead.

This exposed another set of issues:

  1. Ark crashed if closing the UI during a running Job, because result() was emitted and the Part would thing the job succeeded. This is fixed by not emitting finished() from CliInterface. This should definitely fix bug #222392. I'm not sure why this crash doesn't happen on 16.04, probably because of the separate thread.
  2. Ark crashes if canceling the password dialog with header-encrypted archives. I'm working on a separate patch to fix this. (unrelated from this patch).
Test Plan

Close Ark while loading a huge zip. Ark does neither freeze nor crash.

Diff Detail

Repository
R36 Ark
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
elvisangelaccio retitled this revision from to Don't block the main thread while running CliInterface jobs.
elvisangelaccio updated this object.
elvisangelaccio edited the test plan for this revision. (Show Details)
elvisangelaccio added a reviewer: rthomsen.
Restricted Application added a project: Ark. · View Herald TranscriptApr 29 2016, 4:48 PM
Restricted Application added a subscriber: kde-utils-devel. · View Herald Transcript
  1. Ark crashes if canceling the password dialog with header-encrypted archives. I'm working on a separate patch to fix this.

Actually, I can reproduce this crash on current master, so it's unrelated from this patch.

elvisangelaccio updated this object.

Much cleaner fix for the jobs crash.
Instead of the disconnect() workaround, we can avoid to emit finished() in the first place from CliInterface.

rthomsen accepted this revision.Apr 30 2016, 10:52 AM
rthomsen edited edge metadata.

Works for me :)

This revision is now accepted and ready to land.Apr 30 2016, 10:52 AM