poboiko created this revision. poboiko added reviewers: Baloo, bruns, ngraham. Herald added projects: Frameworks, Baloo. poboiko requested review of this revision.
REVISION SUMMARY When a new batch arrives at stdin (as notified by QSocketNotifier), extractor creates a new transaction. If code, which reacts to notification, gets executed multiple times, we end up having two write transaction, which is a bad idea. Current implementation disables `QSocketNotifier`, so that we won't receive new notifications. It blocks further notifications, but it does not eliminate the possibility when several notifications already appeared in the event queue (which can happen due to race condition). It shouldn't happen, normally, but the only guard for that case currently is a single `Q_ASSERT`, which gets silently ignored in the non-debug build. This patch replaces assert with proper check, which instead commits & deletes the transaction if there is any (just in case). It also removes attempts to open the database multiple times (for each batch), which is a bad idea, as LMDB documentation suggests. These attempts will be ignored anyway (because of the check inside Database::open), yet semantically it's better to move the code outside, to the main(), as it is in `baloo_file`. TEST PLAN I didn't manage to cause such race condition "in the laboratory environment", so I've only checked it does not break anything REPOSITORY R293 Baloo BRANCH extractor-transaction (branched from master) REVISION DETAIL https://phabricator.kde.org/D23008 AFFECTED FILES src/file/extractor/app.cpp src/file/extractor/app.h src/file/extractor/main.cpp To: poboiko, #baloo, bruns, ngraham Cc: kde-frameworks-devel, LeGast00n, fbampaloukas, domson, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams