Skip to content

Commit afa51d3

Browse files
committed
Fix Savepoint destructor exception-safety and rollback bookkeeping
The destructor caught only SQLite::Exception, so a std::bad_alloc thrown while building the "ROLLBACK TO SAVEPOINT ..." / "RELEASE SAVEPOINT ..." strings (or any other non-SQLite exception) escaped the implicitly noexcept destructor and called std::terminate. Broaden the handler to catch(...). Track an explicit mbRolledBack state so the destructor issues the minimum commands: when a rollback already happened (manual rollbackTo(), or the scope-exit path), it now does a single RELEASE instead of relying on SQLite tolerating a repeated ROLLBACK TO. The documented auto-rollback semantics are unchanged. Add a regression test covering a manual rollbackTo() followed by release(). Fixes SP-02 and SP-03 from the code review.
1 parent 6c7a96f commit afa51d3

4 files changed

Lines changed: 36 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,3 +309,4 @@ Version 3.4.0 - 2026 ???
309309
- Fix Column::operator<< to stream the exact column bytes via getString() (#556)
310310
- Restore the Coverity Scan static analysis as a GitHub Actions workflow, replacing the old Travis CI job
311311
- Fix Database::getHeaderInfo() signed-shift UB and use fixed-width types for the Header struct (#558)
312+
- Fix Savepoint destructor to catch all exceptions and track rollback state to avoid std::terminate (#559)

include/SQLiteCpp/Savepoint.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -90,9 +90,10 @@ class SQLITECPP_API Savepoint
9090
void rollback() { rollbackTo(); }
9191

9292
private:
93-
Database& mDatabase; ///< Reference to the SQLite Database Connection
94-
std::string msName; ///< Name of the Savepoint
95-
bool mbReleased = false; ///< True when release has been called
93+
Database& mDatabase; ///< Reference to the SQLite Database Connection
94+
std::string msName; ///< Name of the Savepoint
95+
bool mbReleased = false; ///< True when release has been called
96+
bool mbRolledBack = false; ///< True when a rollback to the savepoint has been done
9697
};
9798

9899
} // namespace SQLite

src/Savepoint.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,13 @@ Savepoint::~Savepoint()
4040
{
4141
try
4242
{
43-
rollback();
43+
if (!mbRolledBack)
44+
{
45+
rollbackTo();
46+
}
4447
release();
4548
}
46-
catch (SQLite::Exception&)
49+
catch (...)
4750
{
4851
// Never throw an exception in a destructor: error if already released,
4952
// but no harm is caused by this.
@@ -71,6 +74,7 @@ void Savepoint::rollbackTo()
7174
if (!mbReleased)
7275
{
7376
mDatabase.exec(std::string("ROLLBACK TO SAVEPOINT ") + msName);
77+
mbRolledBack = true;
7478
}
7579
else
7680
{

tests/Savepoint_test.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,31 @@ TEST(Savepoint, commitRollback)
105105
EXPECT_EQ(1, nbRows);
106106
}
107107

108+
TEST(Savepoint, rollbackToThenRelease)
109+
{
110+
// Create a new database
111+
SQLite::Database db(":memory:", SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE);
112+
EXPECT_EQ(SQLite::OK, db.getErrorCode());
113+
db.exec("CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)");
114+
115+
{
116+
SQLite::Savepoint savepoint(db, "sp");
117+
118+
EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'rolled back')"));
119+
120+
// A manual rollback leaves the savepoint on the stack, so releasing it afterwards succeeds.
121+
savepoint.rollbackTo();
122+
EXPECT_NO_THROW(savepoint.release());
123+
124+
// end of scope: already released, the destructor must do nothing and not throw
125+
}
126+
127+
// The rolled-back insert must not be persisted
128+
SQLite::Statement query(db, "SELECT COUNT(*) FROM test");
129+
ASSERT_TRUE(query.executeStep());
130+
EXPECT_EQ(0, query.getColumn(0).getInt());
131+
}
132+
108133
TEST(Savepoint, destructorSwallowsException)
109134
{
110135
// Create a new database

0 commit comments

Comments
 (0)