[Extractor] Replace homegrown IO handler with QDataStream, catch HUP
ClosedPublic

Authored by bruns on Oct 30 2018, 2:23 AM.

Details

Summary

The handler does not handle partial reads, and most importantly, does
not handle when the pipe from the parent process is closed.

Although this should not happen during normal operation, in case of a
crash the indexer process will receive QSocketNotifier::activated events
due to 'POLLHUP' events from the underlying poll. This causes a busy
loop, as the underlying pipe status is never checked.

May fix a few instances of "100% CPU load" bug reports.

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.Oct 30 2018, 2:23 AM
Restricted Application added projects: Frameworks, Baloo. · View Herald TranscriptOct 30 2018, 2:23 AM
Restricted Application added a subscriber: kde-frameworks-devel. · View Herald Transcript
bruns requested review of this revision.Oct 30 2018, 2:23 AM

In general (for my limited Baloo knowledge) this makes sense. You might want to add a few CCBUGs if you are aware of specific bugs this alleviates (@ngraham or someone from the bugsquad may help).

That's nice! I'll test it a little.

src/file/extractor/app.cpp
74

I haven't work with QDataStream before, but shouldn't m_inputStream >> nIds do the same? (and same below)
And probably you should avoid magical constants (4, 8, etc) - better use corresponding sizeofs

bruns added a comment.Oct 30 2018, 1:27 PM

That's nice! I'll test it a little.

This step keeps binary compatibility with the current streaming format. The big cleanup happens in the next commit (D16524),

bruns marked an inline comment as done.Oct 30 2018, 2:38 PM
bruns added inline comments.
src/file/extractor/app.cpp
74

readRawData consumes N bytes, while operator>> consumes N + sizeof(serialization meta data).

I don't care to much for this code, as it is removed in the followup patch.

bruns marked an inline comment as done.Nov 3 2018, 3:23 AM
bruns added a comment.Nov 5 2018, 10:52 PM

good to go?

lbeltrame accepted this revision.Nov 5 2018, 11:00 PM
This revision is now accepted and ready to land.Nov 5 2018, 11:00 PM
This revision was automatically updated to reflect the committed changes.