diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 528972c4..1b3928a8 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -59,7 +59,7 @@ ubuntu: - $INSTALL_CMD deb-package: - stage: build + stage: test image: ubuntu:rolling before_script: - apt-get update diff --git a/src/engine/db/db-database.vala b/src/engine/db/db-database.vala index 90f8f5c2..a13a8777 100644 --- a/src/engine/db/db-database.vala +++ b/src/engine/db/db-database.vala @@ -84,9 +84,9 @@ public class Geary.Db.Database : Geary.Db.Context { } ~Database() { - // Not thrilled about using lock in a dtor - lock (outstanding_async_jobs) { - assert(outstanding_async_jobs == 0); + // Not thrilled about long-running tasks in a dtor + if (this.thread_pool != null) { + GLib.ThreadPool.free((owned) this.thread_pool, true, true); } } diff --git a/src/engine/imap-db/imap-db-database.vala b/src/engine/imap-db/imap-db-database.vala index 807b6c06..f00ff910 100644 --- a/src/engine/imap-db/imap-db-database.vala +++ b/src/engine/imap-db/imap-db-database.vala @@ -17,6 +17,8 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { private ProgressMonitor vacuum_monitor; private string account_owner_email; private bool new_db = false; + + private GC? gc = null; private Cancellable gc_cancellable = new Cancellable(); public Database(GLib.File db_file, @@ -45,21 +47,23 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { // called if (cancellable != null) cancellable.cancelled.connect(cancel_gc); - + // Create new garbage collection object for this database - GC gc = new GC(this, Priority.LOW); - + this.gc = new GC(this, Priority.LOW); + // Get recommendations on what GC operations should be executed - GC.RecommendedOperation recommended = yield gc.should_run_async(gc_cancellable); - + GC.RecommendedOperation recommended = yield this.gc.should_run_async( + gc_cancellable + ); + // VACUUM needs to execute in the foreground with the user given a busy prompt (and cannot // be run at the same time as REAP) if ((recommended & GC.RecommendedOperation.VACUUM) != 0) { if (!vacuum_monitor.is_in_progress) vacuum_monitor.notify_start(); - + try { - yield gc.vacuum_async(gc_cancellable); + yield this.gc.vacuum_async(gc_cancellable); } catch (Error err) { message( "Vacuum of IMAP database %s failed: %s", this.path, err.message @@ -70,35 +74,40 @@ private class Geary.ImapDB.Database : Geary.Db.VersionedDatabase { vacuum_monitor.notify_finish(); } } - + // REAP can run in the background while the application is executing if ((recommended & GC.RecommendedOperation.REAP) != 0) { // run in the background and allow application to continue running - gc.reap_async.begin(gc_cancellable, on_reap_async_completed); + this.gc.reap_async.begin(gc_cancellable, on_reap_async_completed); } - + if (cancellable != null) cancellable.cancelled.disconnect(cancel_gc); } - + private void on_reap_async_completed(Object? object, AsyncResult result) { - GC gc = (GC) object; try { - gc.reap_async.end(result); + this.gc.reap_async.end(result); } catch (Error err) { message("Garbage collection of IMAP database %s failed: %s", this.path, err.message); } + + this.gc = null; } - + private void cancel_gc() { gc_cancellable.cancel(); gc_cancellable = new Cancellable(); } - + public override void close(Cancellable? cancellable) throws Error { + // Ensure GC shuts down before returning cancel_gc(); - + while (this.gc != null && this.gc.is_running) { + GLib.MainContext.default().iteration(false); + } + base.close(cancellable); } diff --git a/src/engine/imap-db/imap-db-gc.vala b/src/engine/imap-db/imap-db-gc.vala index ce52e990..dd170850 100644 --- a/src/engine/imap-db/imap-db-gc.vala +++ b/src/engine/imap-db/imap-db-gc.vala @@ -81,7 +81,7 @@ private class Geary.ImapDB.GC { */ public bool is_running { get; private set; default = false; } - private ImapDB.Database db; + private weak ImapDB.Database db; private int priority; public GC(ImapDB.Database db, int priority) { @@ -107,11 +107,15 @@ private class Geary.ImapDB.GC { (last_vacuum_time != null) ? last_vacuum_time.to_string() : "never", reaped_messages_since_last_vacuum, free_page_bytes.to_string()); - + RecommendedOperation op = RecommendedOperation.NONE; - DateTime now = new DateTime.now_local(); - + if (!yield has_message_rows(cancellable)) { + // No message rows exist, so don't bother vacuuming + return op; + } + // Reap every REAP_DAYS_SPAN unless never executed, in which case run now + DateTime now = new DateTime.now_local(); int64 days; if (last_reap_time == null) { // null means reaping has never executed @@ -613,7 +617,24 @@ private class Geary.ImapDB.GC { return deleted; } - + + private async bool has_message_rows(GLib.Cancellable? cancellable) { + bool ret = false; + yield db.exec_transaction_async(Db.TransactionType.RO, (cx) => { + Db.Result result = cx.query( + "SELECT count(*) FROM MessageTable LIMIT 1" + ); + + Db.TransactionOutcome txn_ret = FAILURE; + if (!result.finished) { + txn_ret = SUCCESS; + ret = result.int64_at(0) > 0; + } + return txn_ret; + }, cancellable); + return ret; + } + private async void fetch_gc_info_async(Cancellable? cancellable, out DateTime? last_reap_time, out DateTime? last_vacuum_time, out int reaped_messages_since_last_vacuum, out int64 free_page_bytes) throws Error { diff --git a/test/engine/imap-db/imap-db-account-test.vala b/test/engine/imap-db/imap-db-account-test.vala index d70e44e5..c6aac242 100644 --- a/test/engine/imap-db/imap-db-account-test.vala +++ b/test/engine/imap-db/imap-db-account-test.vala @@ -30,7 +30,7 @@ class Geary.ImapDB.AccountTest : TestCase { public override void set_up() throws GLib.Error { this.tmp_dir = GLib.File.new_for_path( - GLib.DirUtils.make_tmp("geary-db-database-test-XXXXXX") + GLib.DirUtils.make_tmp("geary-imap-db-account-test-XXXXXX") ); this.config = new Geary.AccountInformation( @@ -59,6 +59,7 @@ class Geary.ImapDB.AccountTest : TestCase { (obj, ret) => { async_complete(ret); } ); this.account.close_async.end(async_result()); + this.account = null; delete_file(this.tmp_dir); this.tmp_dir = null; diff --git a/test/engine/imap-db/imap-db-database-test.vala b/test/engine/imap-db/imap-db-database-test.vala index 892d2815..1f823d6b 100644 --- a/test/engine/imap-db/imap-db-database-test.vala +++ b/test/engine/imap-db/imap-db-database-test.vala @@ -9,21 +9,31 @@ class Geary.ImapDB.DatabaseTest : TestCase { + private GLib.File? tmp_dir = null; + + public DatabaseTest() { base("Geary.ImapDb.DatabaseTest"); add_test("open_new", open_new); add_test("upgrade_0_6", upgrade_0_6); } - public void open_new() throws Error { - GLib.File tmp_dir = GLib.File.new_for_path( - GLib.DirUtils.make_tmp("geary-db-database-test-XXXXXX") + public override void set_up() throws GLib.Error { + this.tmp_dir = GLib.File.new_for_path( + GLib.DirUtils.make_tmp("geary-imap-db-database-test-XXXXXX") ); + } + public override void tear_down() throws GLib.Error { + delete_file(this.tmp_dir); + this.tmp_dir = null; + } + + public void open_new() throws Error { Database db = new Database( - tmp_dir.get_child("test.db"), + this.tmp_dir.get_child("test.db"), GLib.File.new_for_path(_SOURCE_ROOT_DIR).get_child("sql"), - tmp_dir.get_child("attachments"), + this.tmp_dir.get_child("attachments"), new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_UPGRADE), new Geary.SimpleProgressMonitor(Geary.ProgressType.DB_VACUUM), "test@example.com" @@ -41,16 +51,9 @@ class Geary.ImapDB.DatabaseTest : TestCase { // Need to close it again to stop the GC process running db.close(); - - db.file.delete(); - tmp_dir.delete(); } public void upgrade_0_6() throws Error { - GLib.File tmp_dir = GLib.File.new_for_path( - GLib.DirUtils.make_tmp("geary-db-database-test-XXXXXX") - ); - // Since the upgrade process also messes around with // attachments on disk which we want to be able to test, we // need to have a complete-ish database and attachments @@ -65,11 +68,11 @@ class Geary.ImapDB.DatabaseTest : TestCase { GLib.File db_archive = GLib.File .new_for_uri(RESOURCE_URI) .resolve_relative_path(DB_0_6_RESOURCE); - GLib.File db_dir = tmp_dir.get_child(DB_0_6_DIR); + GLib.File db_dir = this.tmp_dir.get_child(DB_0_6_DIR); GLib.File db_file = db_dir.get_child("geary.db"); GLib.File attachments_dir = db_dir.get_child("attachments"); - unpack_archive(db_archive, tmp_dir); + unpack_archive(db_archive, this.tmp_dir); // This number is the id of the last known message in the // database @@ -120,12 +123,6 @@ class Geary.ImapDB.DatabaseTest : TestCase { // Need to close it again to stop the GC process running db.close(); - - Files.recursive_delete_async.begin( - tmp_dir, GLib.Priority.DEFAULT, null, - (obj, res) => { async_complete(res); } - ); - Files.recursive_delete_async.end(async_result()); }