You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2016/04/26 21:04:44 UTC

svn commit: r1741073 - /subversion/trunk/subversion/libsvn_subr/sqlite.c

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);