From 757f6d428cb45a7221d3ab501d9974f78ffe27f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Mon, 10 Jan 2022 09:45:11 +0100 Subject: [PATCH 1/3] MediaLibrary: Don't rely on the database to remove all tables Refs #405 --- src/MediaLibrary.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/src/MediaLibrary.cpp b/src/MediaLibrary.cpp index c3b0a950..7a714362 100644 --- a/src/MediaLibrary.cpp +++ b/src/MediaLibrary.cpp @@ -398,9 +398,59 @@ bool MediaLibrary::createAllTables() void MediaLibrary::deleteAllTables( sqlite::Connection* dbConn ) { - auto tables = sqlite::Tools::listTables( dbConn ); + /* + * For the general case, don't probe the database to delete the active tables. + * If the database is badly corrupted, we might not get any table which would + * cause a failure down the line. + */ + const std::string tables[] = { + Device::Table::Name, + Device::MountpointTable::Name, + Folder::Table::Name, + Folder::FtsTable::Name, + Thumbnail::Table::Name, + Thumbnail::LinkingTable::Name, + Thumbnail::CleanupTable::Name, + Media::Table::Name, + Media::FtsTable::Name, + File::Table::Name, + Label::Table::Name, + Label::FileRelationTable::Name, + Playlist::Table::Name, + Playlist::FtsTable::Name, + Playlist::MediaRelationTable::Name, + Genre::Table::Name, + Genre::FtsTable::Name, + Album::Table::Name, + Album::FtsTable::Name, + Show::Table::Name, + Show::FtsTable::Name, + ShowEpisode::Table::Name, + Movie::Table::Name, + VideoTrack::Table::Name, + AudioTrack::Table::Name, + Artist::Table::Name, + Artist::FtsTable::Name, + Artist::MediaRelationTable::Name, + parser::Task::Table::Name, + Metadata::Table::Name, + SubtitleTrack::Table::Name, + Chapter::Table::Name, + Bookmark::Table::Name, + MediaGroup::Table::Name, + MediaGroup::FtsTable::Name, + "Settings", + }; for ( const auto& table : tables ) + sqlite::Tools::executeRequest( dbConn, "DROP TABLE IF EXISTS " + table ); + /* + * Keep probing the database in case it is quite old and still contains a + * deprecated table not listed above + */ + const auto leftOverTables = sqlite::Tools::listTables( dbConn ); + for ( const auto& table : leftOverTables ) sqlite::Tools::executeRequest( dbConn, "DROP TABLE " + table ); + } void MediaLibrary::createAllTriggers() -- GitLab From 9339f90043ada5000b75fdcdc830ba4501a7b6cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Fri, 14 Jan 2022 13:55:35 +0100 Subject: [PATCH 2/3] TMP: Delete all triggers prior to deleting all the tables --- src/MediaLibrary.cpp | 61 ++++++++++++++++++++++++++++ src/MediaLibrary.h | 1 + test/samples/Tester.cpp | 2 + test/unittest/MediaLibraryTester.cpp | 1 + 4 files changed, 65 insertions(+) diff --git a/src/MediaLibrary.cpp b/src/MediaLibrary.cpp index 7a714362..a82b0f1f 100644 --- a/src/MediaLibrary.cpp +++ b/src/MediaLibrary.cpp @@ -396,6 +396,66 @@ bool MediaLibrary::createAllTables() return true; } +void MediaLibrary::deleteAllTriggers(sqlite::Connection* dbConn) +{ + const std::string triggers[] = { + "album_delete_empty", + "album_delete_track", + "album_is_present", + "artist_decrement_nb_albums", + "artist_decrement_nb_tracks", + "artist_has_tracks_present", + "artist_increment_nb_albums_unknown_album", + "artist_increment_nb_tracks", + "artist_update_nb_albums", + "auto_delete_album_thumbnail", + "auto_delete_artist_thumbnail", "auto_delete_media_thumbnail", + "decr_thumbnail_refcount", + "decrement_media_nb_playlist", + "delete_album_fts", "delete_artist_fts", + "delete_artist_without_tracks", + "delete_folder_fts", "delete_genre_fts", "delete_label_fts", + "delete_media_fts", "delete_playlist_fts", "delete_playlist_linking_tasks", + "delete_show_fts", "delete_unused_thumbnail", + "folder_update_nb_media_on_media_update", + "genre_delete_empty", + "genre_update_is_present", + "genre_update_on_track_deleted", + "incr_thumbnail_refcount", + "increment_media_nb_playlist", "insert_album_fts", "insert_artist_fts", + "insert_folder_fts", "insert_genre_fts", "insert_media_fts", + "insert_playlist_fts", "insert_show_fts", + "media_cascade_file_deletion", + "media_cascade_file_update", + "media_group_decrement_nb_media_on_deletion", + "media_group_delete_empty_group", + "media_group_delete_fts", + "media_group_insert_fts", + "media_group_rename_forced_singleton", + "media_group_update_duration_on_media_change", + "media_group_update_duration_on_media_deletion", + "media_group_update_media_count_on_import_type_change", + "media_group_update_nb_media_types", + "media_group_update_nb_media_types_presence", + "media_update_device_presence", + "playlist_cascade_file_deletion", + "playlist_update_duration_on_media_change", + "playlist_update_nb_media_on_media_change", + "playlist_update_nb_media_on_media_deletion", + "show_decrement_nb_episode", + "show_increment_nb_episode", + "show_update_is_present", + "thumbnail_insert_cleanup", + "update_folder_nb_media_on_delete", + "update_folder_nb_media_on_insert", + "update_media_title_fts", "update_playlist_fts", + "update_playlist_order_on_delete", "update_playlist_order_on_insert", + "update_thumbnail_refcount", + }; + for ( const auto& t : triggers ) + sqlite::Tools::executeRequest( dbConn, "DROP TRIGGER IF EXISTS " + t ); +} + void MediaLibrary::deleteAllTables( sqlite::Connection* dbConn ) { /* @@ -1403,6 +1463,7 @@ bool MediaLibrary::recreateDatabase() { sqlite::Connection::DisableForeignKeyContext ctx{ m_dbConnection.get() }; auto t = m_dbConnection->newTransaction(); + deleteAllTriggers( m_dbConnection.get() ); deleteAllTables( m_dbConnection.get() ); sqlite::Statement::FlushStatementCache(); Settings::createTable( m_dbConnection.get() ); diff --git a/src/MediaLibrary.h b/src/MediaLibrary.h index e5a843a8..91886eac 100644 --- a/src/MediaLibrary.h +++ b/src/MediaLibrary.h @@ -303,6 +303,7 @@ private: protected: virtual void addLocalFsFactory(); + void deleteAllTriggers( sqlite::Connection* dbConn ); void deleteAllTables( medialibrary::sqlite::Connection *dbConn ); protected: diff --git a/test/samples/Tester.cpp b/test/samples/Tester.cpp index bacfc88d..ce268f9d 100644 --- a/test/samples/Tester.cpp +++ b/test/samples/Tester.cpp @@ -251,6 +251,7 @@ class MediaLibraryTester : public MediaLibrary { sqlite::Connection::DisableForeignKeyContext ctx{ m_dbConnection.get() }; auto t = m_dbConnection->newTransaction(); + deleteAllTriggers( m_dbConnection.get() ); deleteAllTables( dbConn ); t->commit(); } @@ -867,6 +868,7 @@ void MediaLibraryResumeTest::onDbConnectionReady( sqlite::Connection *dbConn ) { sqlite::Connection::DisableForeignKeyContext ctx{ m_dbConnection.get() }; auto t = m_dbConnection->newTransaction(); + deleteAllTriggers( m_dbConnection.get() ); deleteAllTables( dbConn ); t->commit(); } diff --git a/test/unittest/MediaLibraryTester.cpp b/test/unittest/MediaLibraryTester.cpp index 0a44cae6..b2c212c5 100644 --- a/test/unittest/MediaLibraryTester.cpp +++ b/test/unittest/MediaLibraryTester.cpp @@ -51,6 +51,7 @@ void MediaLibraryTester::onDbConnectionReady( sqlite::Connection* dbConn ) { sqlite::Connection::WeakDbContext ctx{ m_dbConnection.get() }; auto t = m_dbConnection->newTransaction(); + deleteAllTriggers( m_dbConnection.get() ); deleteAllTables( dbConn ); t->commit(); } -- GitLab From 7062ef24816950ade28b42e5fd948893a8b3c09a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Hugo=20Beauz=C3=A9e-Luyssen?= Date: Mon, 17 Jan 2022 09:36:08 +0100 Subject: [PATCH 3/3] sqlite: Connection: Flush all connections after removing all tables Should fix #405 --- src/MediaLibrary.cpp | 6 ++++++ src/database/SqliteConnection.cpp | 7 +++++++ src/database/SqliteConnection.h | 1 + test/samples/Tester.cpp | 2 ++ test/unittest/MediaLibraryTester.cpp | 1 + 5 files changed, 17 insertions(+) diff --git a/src/MediaLibrary.cpp b/src/MediaLibrary.cpp index a82b0f1f..e54a5f33 100644 --- a/src/MediaLibrary.cpp +++ b/src/MediaLibrary.cpp @@ -1475,6 +1475,12 @@ bool MediaLibrary::recreateDatabase() if ( m_settings.load() == false ) return false; t->commit(); + /* + * Now that we removed all the tables, flush all the connections to avoid + * Database is locked errors. + * See https://www.sqlite.org/c3ref/close.html + */ + m_dbConnection->flushAll(); /* * We just delete all tables but this won't invoke the thumbnails deletion diff --git a/src/database/SqliteConnection.cpp b/src/database/SqliteConnection.cpp index d4534570..b1a82ed9 100644 --- a/src/database/SqliteConnection.cpp +++ b/src/database/SqliteConnection.cpp @@ -259,6 +259,13 @@ const std::string&Connection::dbPath() const return m_dbPath; } +void Connection::flushAll() +{ + Statement::FlushStatementCache(); + std::unique_lock lock( m_connMutex ); + m_conns.clear(); +} + std::shared_ptr Connection::connect( const std::string& dbPath ) { // Use a wrapper to allow make_shared to use the private Connection ctor diff --git a/src/database/SqliteConnection.h b/src/database/SqliteConnection.h index 6ee28eca..67713a4c 100644 --- a/src/database/SqliteConnection.h +++ b/src/database/SqliteConnection.h @@ -90,6 +90,7 @@ public: bool checkSchemaIntegrity(); bool checkForeignKeysIntegrity(); const std::string& dbPath() const; + void flushAll(); static std::shared_ptr connect( const std::string& dbPath ); diff --git a/test/samples/Tester.cpp b/test/samples/Tester.cpp index ce268f9d..b2f5900f 100644 --- a/test/samples/Tester.cpp +++ b/test/samples/Tester.cpp @@ -254,6 +254,7 @@ class MediaLibraryTester : public MediaLibrary deleteAllTriggers( m_dbConnection.get() ); deleteAllTables( dbConn ); t->commit(); + m_dbConnection->flushAll(); } }; } @@ -871,6 +872,7 @@ void MediaLibraryResumeTest::onDbConnectionReady( sqlite::Connection *dbConn ) deleteAllTriggers( m_dbConnection.get() ); deleteAllTables( dbConn ); t->commit(); + m_dbConnection->flushAll(); } parser::Parser* MediaLibraryResumeTest::getParser() const diff --git a/test/unittest/MediaLibraryTester.cpp b/test/unittest/MediaLibraryTester.cpp index b2c212c5..2792c946 100644 --- a/test/unittest/MediaLibraryTester.cpp +++ b/test/unittest/MediaLibraryTester.cpp @@ -54,6 +54,7 @@ void MediaLibraryTester::onDbConnectionReady( sqlite::Connection* dbConn ) deleteAllTriggers( m_dbConnection.get() ); deleteAllTables( dbConn ); t->commit(); + m_dbConnection->flushAll(); } std::shared_ptr MediaLibraryTester::media( int64_t id ) -- GitLab