[PendingFileQueue] Avoid delete + create / create + delete race
ClosedPublic

Authored by bruns on Jun 9 2019, 11:05 PM.

Details

Summary

When the 'delete' flag is set, the file was removed from the index and
the item was removed from the pending file queue (m_cache). When a
'created' event arrives before the queue has been processed, the flag
was merged into the queue item, but never processed (delete + create),
i.e. the file was omitted from the index.

In case a created/modified file was moved to the m_pendingFiles queue
(files to be reindexed later) and deleted, the pending files timer may
fire before process is run, and the indexNewFile/indexModifiedFile
signal is emitted for a no longer existing file. Although this is handled
in the indexer, this causes unnecessary work.

Emit the removeFileIndex immediately and remove the file from all
queues.

A deleted file/folder is kept in the m_recentlyEmitted list, to avoid
excessive events when e.g. a temporary file is created/deleted in short
period of time.

Test Plan

$> date > testfile; rm testfile; date > testfile
After the change, testfile is included in the index

$> date > testfile; date > testfile2; usleep 2000; rm testfile
After the change, indexNewFile signal is emitted only for testfile2,
while for testfile removeFileIndex is emitted.

Diff Detail

Repository
R293 Baloo
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bruns created this revision.Jun 9 2019, 11:05 PM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptJun 9 2019, 11:05 PM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
bruns requested review of this revision.Jun 9 2019, 11:05 PM

LGTM, but maybe an autotest might make sense for this case.

bruns added a comment.EditedJun 13 2019, 11:46 PM

LGTM, but maybe an autotest might make sense for this case.

I have submitted some preliminary changes to make this more feasible. The current autotest structure is not really able to catch this (real timers are problematic, especially when you try to catch race conditions), and any additional test case would add to the test time significantly.

D21790, D21791, D21792, D21793

Awesome, thanks!

ngraham accepted this revision.Jun 15 2019, 12:30 AM
This revision is now accepted and ready to land.Jun 15 2019, 12:30 AM