Fix race condition in LoadJob
ClosedPublic

Authored by elvisangelaccio on Oct 19 2016, 9:40 AM.

Details

Summary

extracttest has random failures when we check properties with the libarchive
plugin. This happens because there is a race condition between
LoadJob::onFinished() (where we read some LoadJob members) and
LoadJob::onNewEntry() (where we write those members).

onFinished() is called when list() returns, in LoadJob::doWork().
But that might happen before onNewEntry() is called, since they are executed
in two different threads.

This patch puts the onFinished() call in the event queue, just like the
single-thread case does (where we emit the finished signal from
CliInterface).

An alternative could be using a DirectConnection for the entry signal, but
that is more invasive/ugly.

Test Plan

Run extracttest a bunch of times, with/without the patch. It should randomly
fail without the patch, it should always pass with the patch.

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 Fix race condition in LoadJob.
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 TranscriptOct 19 2016, 9:40 AM
Restricted Application added a subscriber: kde-utils-devel. · View Herald Transcript
rthomsen edited edge metadata.Oct 20 2016, 4:45 PM

Fixes ExtractTest for me :)

rthomsen accepted this revision.Oct 20 2016, 4:45 PM
rthomsen edited edge metadata.
This revision is now accepted and ready to land.Oct 20 2016, 4:45 PM
This revision was automatically updated to reflect the committed changes.