D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:145caeac1050: [Bookmarks Runner] Open database connection 
in the query thread (authored by bruns).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16409?vs=44169&id=45489

REVISION DETAIL
  https://phabricator.kde.org/D16409

AFFECTED FILES
  runners/bookmarks/fetchsqlite.cpp
  runners/bookmarks/fetchsqlite.h

To: bruns, #frameworks, mart
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

bruns edited the summary of this revision.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D16409

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

bruns added a dependent revision: D16410: [Bookmarks Runner] Avoid leaking 
FetchSqlite instances.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D16409

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

bruns added a dependency: D16405: Add debug category for bookmarks runner.

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D16409

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16409: [Bookmarks Runner] Open database connection in the query thread

[Stefan Brüns]

bruns created this revision.
bruns added a reviewer: Frameworks.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
bruns requested review of this revision.

REVISION SUMMARY
  QSqlDatabase connection instances are global reference counted objects which
  can be crated using the QSqlDatabase::addDatabase(...) method and later
  retrieved with QSqlDatabase::database(connectionname).
  
  Connections should only be used from a single thread, and should be cleaned
  up with QSD::removeDatabase(name), which implicitly closes the connection
  when the last reference is removed.
  
  Currently, the same connection (name) is reused over all threads, and
  the connection is only removed implicitly by replacing it via addDatabase().
  This leads to warnings, i.e.: "QSqlDatabasePrivate::addDatabase: duplicate
  connection name '...', old connection removed."
  
  As one reference is held by the m_db member, the implicit removal triggers
  another warning: "QSqlDatabasePrivate::removeDatabase: connection '..."
  is still in use, all queries will cease to work."
  
  According to the documentation, "It is highly recommended that you do not
  keep a copy of the QSqlDatabase around as a member of a class, as this
  will prevent the instance from being correctly cleaned up on shutdown."
  There is no need to keep a reference using a member variable, as retrieving
  an open connection via QSqlDatabase::database(...) is cheap.
  
  Create a per-thread DB connection on first use, and remove the connections
  when the FetchSqlite instance is torn down.
  
  Delaying the open is beneficial for the favicon instance, which may be
  unused when the icon is already cached on disk.
  
  See also T9626 .

TEST PLAN
  execute a query in krunner
  
  1. Results are as expected
  2. The 'QSqlDatabasePrivate::removeDatabase: connection ... is still in use, 
all queries will cease to work.' warning no longer appears
  3. The 'QSqlDatabasePrivate::addDatabase: duplicate connection name ..., old 
connection removed' warning no longer appears

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D16409

AFFECTED FILES
  runners/bookmarks/fetchsqlite.cpp
  runners/bookmarks/fetchsqlite.h

To: bruns, #frameworks
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart