diff options
Diffstat (limited to 'subversion/libsvn_subr/sqlite.c')
| -rw-r--r-- | subversion/libsvn_subr/sqlite.c | 142 |
1 files changed, 93 insertions, 49 deletions
diff --git a/subversion/libsvn_subr/sqlite.c b/subversion/libsvn_subr/sqlite.c index 18d1c4928558..18124a3508e6 100644 --- a/subversion/libsvn_subr/sqlite.c +++ b/subversion/libsvn_subr/sqlite.c @@ -1261,6 +1261,55 @@ reset_all_statements(svn_sqlite__db_t *db, return err; } +static svn_error_t * +rollback_transaction(svn_sqlite__db_t *db, + svn_error_t *error_to_wrap) +{ + svn_sqlite__stmt_t *stmt; + svn_error_t *err; + + err = get_internal_statement(&stmt, db, STMT_INTERNAL_ROLLBACK_TRANSACTION); + if (!err) + { + err = svn_error_trace(svn_sqlite__step_done(stmt)); + + if (err && err->apr_err == SVN_ERR_SQLITE_BUSY) + { + /* ### Houston, we have a problem! + + We are trying to rollback but we can't because some + statements are still busy. This leaves the database + unusable for future transactions as the current transaction + is still open. + + As we are returning the actual error as the most relevant + error in the chain, our caller might assume that it can + retry/compensate on this error (e.g. SVN_WC_LOCKED), while + in fact the SQLite database is unusable until the statements + started within this transaction are reset and the transaction + aborted. + + We try to compensate by resetting all prepared but unreset + statements; but we leave the busy error in the chain anyway to + help diagnosing the original error and help in finding where + a reset statement is missing. */ + err = svn_error_trace(reset_all_statements(db, err)); + err = svn_error_compose_create( + svn_error_trace(svn_sqlite__step_done(stmt)), + err); + } + } + + if (err) + { + /* Rollback failed, use a specific error code. */ + err = svn_error_create(SVN_SQLITE__ERR_ROLLBACK_FAILED, err, + _("SQLite transaction rollback failed")); + } + + return svn_error_compose_create(error_to_wrap, err); +} + svn_error_t * svn_sqlite__begin_transaction(svn_sqlite__db_t *db) { @@ -1303,46 +1352,37 @@ svn_sqlite__finish_transaction(svn_sqlite__db_t *db, /* Commit or rollback the sqlite transaction. */ if (err) { - svn_error_t *err2; - - err2 = get_internal_statement(&stmt, db, - STMT_INTERNAL_ROLLBACK_TRANSACTION); - if (!err2) - err2 = svn_sqlite__step_done(stmt); - - if (err2 && err2->apr_err == SVN_ERR_SQLITE_BUSY) - { - /* ### Houston, we have a problem! - - We are trying to rollback but we can't because some - statements are still busy. This leaves the database - unusable for future transactions as the current transaction - is still open. - - As we are returning the actual error as the most relevant - error in the chain, our caller might assume that it can - retry/compensate on this error (e.g. SVN_WC_LOCKED), while - in fact the SQLite database is unusable until the statements - started within this transaction are reset and the transaction - aborted. - - We try to compensate by resetting all prepared but unreset - statements; but we leave the busy error in the chain anyway to - help diagnosing the original error and help in finding where - a reset statement is missing. */ - - err2 = reset_all_statements(db, err2); - err2 = svn_error_compose_create( - svn_sqlite__step_done(stmt), - err2); - } - - return svn_error_compose_create(err, - err2); + return svn_error_trace(rollback_transaction(db, err)); + } + else + { + err = get_internal_statement(&stmt, db, + STMT_INTERNAL_COMMIT_TRANSACTION); + if (!err) + err = svn_error_trace(svn_sqlite__step_done(stmt)); + + /* Need to rollback if the commit fails as well, because otherwise the + db connection will be left in an unusable state. + + One important case to keep in mind is trying to COMMIT with concurrent + readers. In case the commit fails, because someone else is holding a + shared lock, sqlite keeps the transaction, and *also* keeps the file + locks on the database. While the first part only prevents from using + this connection, the second part prevents everyone else from accessing + the database while the connection is open. + + See https://www.sqlite.org/lang_transaction.html + + COMMIT might also result in an SQLITE_BUSY return code if an another + thread or process has a shared lock on the database that prevented + the database from being updated. When COMMIT fails in this way, the + transaction remains active and the COMMIT can be retried later after + the reader has had a chance to clear. */ + if (err) + return svn_error_trace(rollback_transaction(db, err)); } - SVN_ERR(get_internal_statement(&stmt, db, STMT_INTERNAL_COMMIT_TRANSACTION)); - return svn_error_trace(svn_sqlite__step_done(stmt)); + return SVN_NO_ERROR; } svn_error_t * @@ -1359,18 +1399,22 @@ svn_sqlite__finish_savepoint(svn_sqlite__db_t *db, STMT_INTERNAL_ROLLBACK_TO_SAVEPOINT_SVN); if (!err2) - err2 = svn_sqlite__step_done(stmt); - - if (err2 && err2->apr_err == SVN_ERR_SQLITE_BUSY) { - /* Ok, we have a major problem. Some statement is still open, which - makes it impossible to release this savepoint. + err2 = svn_error_trace(svn_sqlite__step_done(stmt)); + + if (err2 && err2->apr_err == SVN_ERR_SQLITE_BUSY) + { + /* Ok, we have a major problem. Some statement is still open, + which makes it impossible to release this savepoint. - ### See huge comment in svn_sqlite__finish_transaction for - further details */ + ### See huge comment in svn_sqlite__finish_transaction for + further details */ - err2 = reset_all_statements(db, err2); - err2 = svn_error_compose_create(svn_sqlite__step_done(stmt), err2); + err2 = svn_error_trace(reset_all_statements(db, err2)); + err2 = svn_error_compose_create( + svn_error_trace(svn_sqlite__step_done(stmt)), + err2); + } } err = svn_error_compose_create(err, err2); @@ -1378,9 +1422,9 @@ svn_sqlite__finish_savepoint(svn_sqlite__db_t *db, STMT_INTERNAL_RELEASE_SAVEPOINT_SVN); if (!err2) - err2 = svn_sqlite__step_done(stmt); + err2 = svn_error_trace(svn_sqlite__step_done(stmt)); - return svn_error_trace(svn_error_compose_create(err, err2)); + return svn_error_compose_create(err, err2); } SVN_ERR(get_internal_statement(&stmt, db, |