activities: Close the database connection to prevent file descriptor leak
ClosedPublic

Authored by akandaurov on Apr 11 2020, 8:08 AM.

Details

Summary

The connection to the database doesn't get closed, which leads to a leakage of file descriptors to kactivities database files, eventually reaching the open file limit. Also, sometimes warnings about duplicate connections may appear in the console. This patch fixes this by closing and removing the connection.

Test Plan
  1. Right-click a file in Dolphin and hover the Activities menu. Do it several times.
  2. Check the output of
ls -l /proc/`pidof dolphin`/fd

for open descriptors to ~/.local/share/kactivitymanagerd/resources/database{,-wal}.

Diff Detail

Repository
R320 KIO Extras
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
akandaurov created this revision.Apr 11 2020, 8:08 AM
Restricted Application added projects: Dolphin, Frameworks. · View Herald TranscriptApr 11 2020, 8:08 AM
Restricted Application added subscribers: kfm-devel, kde-frameworks-devel. · View Herald Transcript
akandaurov requested review of this revision.Apr 11 2020, 8:08 AM
meven requested changes to this revision.Apr 11 2020, 10:02 AM
meven added a subscriber: meven.

Nice patch

activities/fileitemplugin/FileItemLinkingPluginActionLoader.cpp
88

Do you really need to have block opened here and close line 160 ?
I would favor not adding one as it seems not necessary, (or adapt the indentation according to the new block)

This revision now requires changes to proceed.Apr 11 2020, 10:02 AM

Added indentation. The block is suggested by the Qt Documentation. Without it, a warning will be thrown:

QSqlDatabasePrivate::removeDatabase: connection 'kactivities_db_resources_94156823977952' is still in use, all queries will cease to work.
akandaurov marked an inline comment as done.Apr 11 2020, 12:47 PM
meven accepted this revision.Apr 11 2020, 1:25 PM

Too bad we have so many lines to change.

Could you maybe add some context to your commit comment like activities: Close the database connection to prevent file descriptor leak

LGTM

This revision is now accepted and ready to land.Apr 11 2020, 1:25 PM
akandaurov retitled this revision from Close the database connection to prevent file descriptor leak to activities: Close the database connection to prevent file descriptor leak.Apr 11 2020, 1:27 PM
meven added a comment.Apr 11 2020, 3:12 PM

Do you want me to land the commit ?
I believe you don't have a KDE Developer account.

Yes, I don't have an account, so I'll need someone to land it, thanks.

This revision was automatically updated to reflect the committed changes.
meven added a comment.Apr 11 2020, 4:33 PM

Well done @akandaurov
Keep the patch coming ;-)