Author: kotkov Date: Tue Apr 26 19:04:43 2016 New Revision: 1741073 URL: http://svn.apache.org/viewvc?rev=1741073&view=rev Log: Fix a potential crash in a couple of our sqlite helpers.
* subversion/libsvn_subr/sqlite.c (svn_sqlite__finish_transaction, svn_sqlite__finish_savepoint): Don't try to step the statement in case get_internal_statement() returns SVN_ERR_SQLITE_BUSY. If that is the case, the statement variable would be uninitialized, but we'd still try to access it. Modified: subversion/trunk/subversion/libsvn_subr/sqlite.c Modified: subversion/trunk/subversion/libsvn_subr/sqlite.c URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/sqlite.c?rev=1741073&r1=1741072&r2=1741073&view=diff ============================================================================== --- subversion/trunk/subversion/libsvn_subr/sqlite.c (original) +++ subversion/trunk/subversion/libsvn_subr/sqlite.c Tue Apr 26 19:04:43 2016 @@ -1308,35 +1308,37 @@ svn_sqlite__finish_transaction(svn_sqlit err2 = get_internal_statement(&stmt, db, STMT_INTERNAL_ROLLBACK_TRANSACTION); if (!err2) - err2 = svn_error_trace(svn_sqlite__step_done(stmt)); - - if (err2 && err2->apr_err == SVN_ERR_SQLITE_BUSY) { - /* ### Houston, we have a problem! + err2 = svn_error_trace(svn_sqlite__step_done(stmt)); - 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 = svn_error_trace(reset_all_statements(db, err2)); - err2 = svn_error_compose_create( - svn_error_trace(svn_sqlite__step_done(stmt)), - err2); + 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 = svn_error_trace(reset_all_statements(db, err2)); + err2 = svn_error_compose_create( + svn_error_trace(svn_sqlite__step_done(stmt)), + err2); - } + } + } return svn_error_compose_create(err, err2); } @@ -1359,20 +1361,22 @@ svn_sqlite__finish_savepoint(svn_sqlite_ STMT_INTERNAL_ROLLBACK_TO_SAVEPOINT_SVN); if (!err2) - 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 */ + err2 = svn_error_trace(svn_sqlite__step_done(stmt)); - err2 = svn_error_trace(reset_all_statements(db, err2)); - err2 = svn_error_compose_create( - svn_error_trace(svn_sqlite__step_done(stmt)), - err2); + 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 */ + + 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);