Siegfried Gevatter has proposed merging lp:~rainct/zeitgeist/961974 into lp:zeitgeist.
Requested reviews: Zeitgeist Framework Team (zeitgeist) Related bugs: Bug #961974 in Zeitgeist Framework: "Recover from Zeitgeist database corruption (detected at query time)" https://bugs.launchpad.net/zeitgeist/+bug/961974 For more details, see: https://code.launchpad.net/~rainct/zeitgeist/961974/+merge/100946 -- https://code.launchpad.net/~rainct/zeitgeist/961974/+merge/100946 Your team Zeitgeist Framework Team is requested to review the proposed merge of lp:~rainct/zeitgeist/961974 into lp:zeitgeist.
=== modified file 'extensions/histogram.vala' --- extensions/histogram.vala 2011-12-07 12:17:29 +0000 +++ extensions/histogram.vala 2012-04-05 12:15:23 +0000 @@ -96,14 +96,8 @@ builder.add ("(xu)", t, count); } - - if (rc != Sqlite.DONE) - { - string error_message = "Error in get_histogram_data: " + - "%d, %s".printf (rc, db.errmsg ()); - warning ("%s", error_message); - throw new EngineError.DATABASE_ERROR (error_message); - } + database.assert_query_success (rc, "Error in get_histogram_data", + Sqlite.DONE); return builder.end (); } === modified file 'src/db-reader.vala' --- src/db-reader.vala 2012-03-19 19:56:38 +0000 +++ src/db-reader.vala 2012-04-05 12:15:23 +0000 @@ -55,13 +55,22 @@ database.set_deletion_callback (delete_from_cache); db = database.database; - interpretations_table = new TableLookup (database, "interpretation"); - manifestations_table = new TableLookup (database, "manifestation"); - mimetypes_table = new TableLookup (database, "mimetype"); - actors_table = new TableLookup (database, "actor"); + try + { + interpretations_table = new TableLookup (database, "interpretation"); + manifestations_table = new TableLookup (database, "manifestation"); + mimetypes_table = new TableLookup (database, "mimetype"); + actors_table = new TableLookup (database, "actor"); + } + catch (EngineError err) + { + // FIXME: propagate this properly? + critical ("TableLookup initialization failed: %s", err.message); + } } protected Event get_event_from_row (Sqlite.Statement stmt, uint32 event_id) + throws EngineError { Event event = new Event (); event.id = event_id; @@ -88,6 +97,7 @@ } protected Subject get_subject_from_row (Sqlite.Statement stmt) + throws EngineError { Subject subject = new Subject (); subject.uri = stmt.column_text (EventViewRows.SUBJECT_URI); @@ -142,11 +152,7 @@ Subject subject = get_subject_from_row(stmt); event.add_subject(subject); } - if (rc != Sqlite.DONE) - { - throw new EngineError.DATABASE_ERROR ("Error: %d, %s\n", - rc, db.errmsg ()); - } + database.assert_query_success (rc, "Error", Sqlite.DONE); // Sort events according to the sequence of event_ids var results = new GenericArray<Event?> (); @@ -270,7 +276,7 @@ warning (error_message); throw new EngineError.INVALID_ARGUMENT (error_message); } - + // complete the sort rule bool time_asc = ResultType.is_sort_order_asc ((ResultType) result_type); sql += " timestamp %s".printf ((time_asc) ? "ASC" : "DESC"); @@ -306,6 +312,7 @@ string error_message = "Error in find_event_ids: %d, %s".printf ( rc, db.errmsg ()); warning (error_message); + database.assert_not_corrupt (rc); throw new EngineError.DATABASE_ERROR (error_message); } @@ -458,14 +465,7 @@ // for (int i=0; i<related_uris.length; i++) // related_uris[i] = temp_related_uris[i]; - if (rc != Sqlite.DONE) - { - string error_message = - "Error in find_related_uris: %d, %s".printf ( - rc, db.errmsg ()); - warning (error_message); - throw new EngineError.DATABASE_ERROR (error_message); - } + database.assert_query_success (rc, "Error in find_related_uris"); var uri_counter = new HashTable<string, RelatedUri?>( str_hash, str_equal); === modified file 'src/engine.vala' --- src/engine.vala 2012-03-14 14:26:11 +0000 +++ src/engine.vala 2012-04-05 12:15:23 +0000 @@ -241,6 +241,7 @@ if ((rc = insert_stmt.step()) != Sqlite.DONE) { if (rc != Sqlite.CONSTRAINT) { + database.assert_not_corrupt (rc); warning ("SQL error: %d, %s\n", rc, db.errmsg ()); return 0; } @@ -261,6 +262,7 @@ retrieval_stmt.bind_int64 (4, actors_table.id_for_string (event.actor)); if ((rc = retrieval_stmt.step ()) != Sqlite.ROW) { + database.assert_not_corrupt (rc); warning ("SQL error: %d, %s\n", rc, db.errmsg ()); return 0; } @@ -340,6 +342,11 @@ if ((rc = move_stmt.step()) != Sqlite.DONE) { if (rc != Sqlite.CONSTRAINT) { + try + { + database.assert_not_corrupt (rc); + } + catch (EngineError err) {} warning ("SQL error: %d, %s\n", rc, db.errmsg ()); } } @@ -364,7 +371,14 @@ event.payload.data.length); if ((rc = payload_insertion_stmt.step ()) != Sqlite.DONE) if (rc != Sqlite.CONSTRAINT) + { warning ("SQL error: %d, %s\n", rc, db.errmsg ()); + try + { + database.assert_not_corrupt (rc); + } + catch (EngineError err) { } + } return database.database.last_insert_rowid (); } === modified file 'src/extension-store.vala' --- src/extension-store.vala 2012-02-02 18:57:35 +0000 +++ src/extension-store.vala 2012-04-05 12:15:23 +0000 @@ -81,13 +81,20 @@ storage_stmt.bind_blob (3, data.get_data (), (int) data.get_size ()); if ((rc = storage_stmt.step ()) != Sqlite.DONE) + { + try + { + database.assert_not_corrupt (rc); + } + catch (EngineError err) { } warning ("SQL error: %d, %s", rc, db.errmsg ()); + } } /** * Retrieve a previously stored value. */ - public Variant? retrieve(string extension, string key, VariantType format) + public Variant? retrieve (string extension, string key, VariantType format) { retrieval_stmt.reset (); retrieval_stmt.bind_text (1, extension); @@ -97,7 +104,14 @@ if (rc != Sqlite.ROW) { if (rc != Sqlite.DONE) + { + try + { + database.assert_not_corrupt (rc); + } + catch (EngineError) { } warning ("SQL error: %d, %s", rc, db.errmsg ()); + } return null; } === modified file 'src/sql-schema.vala' --- src/sql-schema.vala 2012-02-10 16:52:57 +0000 +++ src/sql-schema.vala 2012-04-05 12:15:23 +0000 @@ -2,8 +2,9 @@ * * Copyright © 2011 Collabora Ltd. * By Siegfried-Angel Gevatter Pujals <siegfr...@gevatter.com> - * Copyright © 2011 Canonical Ltd. + * Copyright © 2011-2012 Canonical Ltd. * By Michal Hruby <michal.hr...@canonical.com> + * By Siegfried-A. Gevatter <siegfried.gevat...@collabora.co.uk> * * Based upon a Python implementation (2009-2011) by: * Markus Korn <thek...@gmx.net> @@ -120,6 +121,7 @@ } public static int get_schema_version (Sqlite.Database database) + throws EngineError { var sql = "SELECT version FROM schema_version WHERE schema='core'"; int schema_version = -1; @@ -137,9 +139,35 @@ // will be -1 if something went wrong anyway debug ("schema_version is %d", schema_version); + if (schema_version < -1) + { + throw new EngineError.DATABASE_CORRUPT ( + "Database corruption flag is set."); + } return schema_version; } + public static void set_corruption_flag (Sqlite.Database database) + throws EngineError + { + // A schema_version value smaller than -1 indicates that + // database corruption has been detected. + int version = get_schema_version (database); + if (version > 0) + version = -version; + set_schema_version (database, version); + } + + private static void set_schema_version (Sqlite.Database database, + int schema_version) throws EngineError + { + /* The 'ON CONFLICT REPLACE' on the PK converts INSERT to UPDATE + * when appriopriate */ + var schema_sql = "INSERT INTO schema_version VALUES ('%s', %d)" + .printf (CORE_SCHEMA, schema_version); + exec_query (database, schema_sql); + } + public static void create_schema (Sqlite.Database database) throws EngineError { @@ -458,13 +486,7 @@ version INT ) """); - - /* The 'ON CONFLICT REPLACE' on the PK converts INSERT to UPDATE - * when appriopriate */ - var schema_sql = "INSERT INTO schema_version VALUES ('%s', %d)" - .printf (CORE_SCHEMA, CORE_SCHEMA_VERSION); - exec_query (database, schema_sql); - + set_schema_version (database, CORE_SCHEMA_VERSION); } /** === modified file 'src/sql.vala' --- src/sql.vala 2012-03-29 17:51:33 +0000 +++ src/sql.vala 2012-04-05 12:15:23 +0000 @@ -4,6 +4,8 @@ * By Siegfried-Angel Gevatter Pujals <siegfr...@gevatter.com> * By Seif Lotfy <s...@lotfy.com> * Copyright © 2011 Manish Sinha <manishsi...@ubuntu.com> + * Copyright © 2012 Canonical Ltd. + * By Siegfried-A. Gevatter <siegfried.gevat...@collabora.co.uk> * * This program is free software: you can redistribute it and/or modify * it under the terms of the GNU Lesser General Public License as published by @@ -106,7 +108,7 @@ { try { - // Error (like a malformed database) may not be exposed + // Errors (like a malformed database) may not be exposed // until we try to operate on the database. if (is_read_only) { @@ -142,17 +144,7 @@ // The database disk image is malformed warning ("It looks like your database is corrupt. " + "It will be renamed and a new one will be created."); - try - { - Utils.retire_database (); - } - catch (Error err) - { - string message = - "Could not rename database: %s".printf ( - err.message); - throw new EngineError.DATABASE_RETIRE_FAILED (message); - } + Utils.retire_database (); open_database (false); } else if (rc == Sqlite.PERM || rc == Sqlite.CANTOPEN) @@ -329,10 +321,40 @@ string error_message = "%s: %d, %s".printf ( msg, rc, database.errmsg ()); warning ("%s\n", error_message); + assert_not_corrupt (rc); throw new EngineError.DATABASE_ERROR (error_message); } } + /** + * Ensure `rc' isn't SQLITE_CORRUPT. If it is, schedule a database + * retire and Zeitgeist restart so a new database can be created, + * unless in read-only mode, in which case EngineError.DATABASE_ERROR + * will be thrown. + * + * This function should be called whenever assert_query_success isn't + * used. + * + * @param rc error code returned by a SQLite call + */ + public void assert_not_corrupt (int rc) + throws EngineError + { + if (unlikely (rc == Sqlite.CORRUPT)) + { + warning ("It looks like your database is corrupt: %s".printf ( + database.errmsg ())); + if (!is_read_only) + { + // Sets a flag in the database indicating that it is + // corrupt. This will trigger a database retire and + // re-creation on the next startup. + DatabaseSchema.set_corruption_flag (database); + } + throw new EngineError.DATABASE_CORRUPT (database.errmsg ()); + } + } + private void prepare_read_queries () throws EngineError { int rc; === modified file 'src/table-lookup.vala' --- src/table-lookup.vala 2012-03-19 19:12:41 +0000 +++ src/table-lookup.vala 2012-04-05 12:15:23 +0000 @@ -26,7 +26,7 @@ public class TableLookup : Object { - + unowned Zeitgeist.SQLite.Database database; unowned Sqlite.Database db; private string table; @@ -36,7 +36,9 @@ private Sqlite.Statement retrieval_stmt; public TableLookup (Database database, string table_name) + throws EngineError { + this.database = database; db = database.database; table = table_name; id_to_value = new HashTable<int, string>(direct_hash, direct_equal); @@ -52,27 +54,16 @@ value_to_id.insert (values[1], int.parse(values[0])); return 0; }, null); - if (rc != Sqlite.OK) - { - critical ("Can't init %s table: %d, %s\n", table, - rc, db.errmsg ()); - } + database.assert_query_success (rc, + "Can't init %s table".printf (table)); sql = "INSERT INTO " + table + " (value) VALUES (?)"; rc = db.prepare_v2 (sql, -1, out insertion_stmt); - if (rc != Sqlite.OK) - { - // FIXME: throw exception and propagate it up to - // zeitgeist-daemon to abort with DB error? - critical ("SQL error: %d, %s\n", rc, db.errmsg ()); - } + database.assert_query_success (rc, "Error creating insertion_stmt"); sql = "SELECT value FROM " + table + " WHERE id=?"; rc = db.prepare_v2 (sql, -1, out retrieval_stmt); - if (rc != Sqlite.OK) - { - critical ("SQL error: %d, %s\n", rc, db.errmsg ()); - } + database.assert_query_success (rc, "Error creating retrieval_stmt"); } /** @@ -94,7 +85,7 @@ * @see id_try_string * */ - public int id_for_string (string name) + public int id_for_string (string name) throws EngineError { int id = value_to_id.lookup (name); if (id == 0) @@ -102,10 +93,8 @@ int rc; insertion_stmt.reset (); insertion_stmt.bind_text (1, name); - if ((rc = insertion_stmt.step ()) != Sqlite.DONE) - { - critical ("SQL error: %d, %s\n", rc, db.errmsg ()); - } + rc = insertion_stmt.step (); + database.assert_query_success (rc, "Error in id_for_string"); id = (int) db.last_insert_rowid (); @@ -115,7 +104,7 @@ return id; } - public unowned string get_value (int id) + public unowned string get_value (int id) throws EngineError { // When we fetch an event, it either was already in the database // at the time Zeitgeist started or it was inserted later -using @@ -138,7 +127,8 @@ value_to_id.insert (text, id); rc = retrieval_stmt.step (); } - if (rc != Sqlite.DONE || text == null) + database.assert_query_success (rc, "Error in get_value"); + if (text == null) { critical ("Error getting data from table: %d, %s\n", rc, db.errmsg ()); === modified file 'src/utils.vala' --- src/utils.vala 2012-03-26 15:03:10 +0000 +++ src/utils.vala 2012-04-05 12:15:23 +0000 @@ -130,10 +130,19 @@ original.copy (destination, FileCopyFlags.OVERWRITE, null, null); } - public void retire_database () throws Error + public void retire_database () throws EngineError { - File dbfile = File.new_for_path (get_database_file_path ()); - dbfile.set_display_name (get_database_file_retire_name ()); + try + { + File dbfile = File.new_for_path (get_database_file_path ()); + dbfile.set_display_name (get_database_file_retire_name ()); + } + catch (Error err) + { + string message = "Could not rename database: %s".printf ( + err.message); + throw new EngineError.DATABASE_RETIRE_FAILED (message); + } } /**
_______________________________________________ Mailing list: https://launchpad.net/~zeitgeist Post to : zeitgeist@lists.launchpad.net Unsubscribe : https://launchpad.net/~zeitgeist More help : https://help.launchpad.net/ListHelp