From afa51d34285edc373014f8d94e5844756600113a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Rombauts?= Date: Tue, 30 Jun 2026 19:07:26 +0200 Subject: [PATCH 1/2] 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. --- CHANGELOG.md | 1 + include/SQLiteCpp/Savepoint.h | 7 ++++--- src/Savepoint.cpp | 8 ++++++-- tests/Savepoint_test.cpp | 25 +++++++++++++++++++++++++ 4 files changed, 36 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7fb52662..562ac344 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -309,3 +309,4 @@ Version 3.4.0 - 2026 ??? - Fix Column::operator<< to stream the exact column bytes via getString() (#556) - Restore the Coverity Scan static analysis as a GitHub Actions workflow, replacing the old Travis CI job - Fix Database::getHeaderInfo() signed-shift UB and use fixed-width types for the Header struct (#558) +- Fix Savepoint destructor to catch all exceptions and track rollback state to avoid std::terminate (#559) diff --git a/include/SQLiteCpp/Savepoint.h b/include/SQLiteCpp/Savepoint.h index 180f8ac4..f0b67701 100644 --- a/include/SQLiteCpp/Savepoint.h +++ b/include/SQLiteCpp/Savepoint.h @@ -90,9 +90,10 @@ class SQLITECPP_API Savepoint void rollback() { rollbackTo(); } private: - Database& mDatabase; ///< Reference to the SQLite Database Connection - std::string msName; ///< Name of the Savepoint - bool mbReleased = false; ///< True when release has been called + Database& mDatabase; ///< Reference to the SQLite Database Connection + std::string msName; ///< Name of the Savepoint + bool mbReleased = false; ///< True when release has been called + bool mbRolledBack = false; ///< True when a rollback to the savepoint has been done }; } // namespace SQLite diff --git a/src/Savepoint.cpp b/src/Savepoint.cpp index 5bdd0a8d..b0fa9e76 100644 --- a/src/Savepoint.cpp +++ b/src/Savepoint.cpp @@ -40,10 +40,13 @@ Savepoint::~Savepoint() { try { - rollback(); + if (!mbRolledBack) + { + rollbackTo(); + } release(); } - catch (SQLite::Exception&) + catch (...) { // Never throw an exception in a destructor: error if already released, // but no harm is caused by this. @@ -71,6 +74,7 @@ void Savepoint::rollbackTo() if (!mbReleased) { mDatabase.exec(std::string("ROLLBACK TO SAVEPOINT ") + msName); + mbRolledBack = true; } else { diff --git a/tests/Savepoint_test.cpp b/tests/Savepoint_test.cpp index 082d5f34..a5c39423 100644 --- a/tests/Savepoint_test.cpp +++ b/tests/Savepoint_test.cpp @@ -105,6 +105,31 @@ TEST(Savepoint, commitRollback) EXPECT_EQ(1, nbRows); } +TEST(Savepoint, rollbackToThenRelease) +{ + // Create a new database + SQLite::Database db(":memory:", SQLite::OPEN_READWRITE | SQLite::OPEN_CREATE); + EXPECT_EQ(SQLite::OK, db.getErrorCode()); + db.exec("CREATE TABLE test (id INTEGER PRIMARY KEY, value TEXT)"); + + { + SQLite::Savepoint savepoint(db, "sp"); + + EXPECT_EQ(1, db.exec("INSERT INTO test VALUES (NULL, 'rolled back')")); + + // A manual rollback leaves the savepoint on the stack, so releasing it afterwards succeeds. + savepoint.rollbackTo(); + EXPECT_NO_THROW(savepoint.release()); + + // end of scope: already released, the destructor must do nothing and not throw + } + + // The rolled-back insert must not be persisted + SQLite::Statement query(db, "SELECT COUNT(*) FROM test"); + ASSERT_TRUE(query.executeStep()); + EXPECT_EQ(0, query.getColumn(0).getInt()); +} + TEST(Savepoint, destructorSwallowsException) { // Create a new database From 5e4908ab4b6c675b7afb94aafc30a234c0f2f356 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Rombauts?= Date: Wed, 1 Jul 2026 01:30:44 +0200 Subject: [PATCH 2/2] Fix Transaction destructor exception-safety The destructor caught only SQLite::Exception, so a std::bad_alloc thrown while building the "ROLLBACK TRANSACTION" string (or any other non-SQLite exception) escaped the implicitly noexcept destructor and called std::terminate. Broaden the handler to catch(...), matching the Savepoint fix (afa51d3). Fixes TXN-08 from the code review. --- CHANGELOG.md | 1 + src/Transaction.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 562ac344..374dd5cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -310,3 +310,4 @@ Version 3.4.0 - 2026 ??? - Restore the Coverity Scan static analysis as a GitHub Actions workflow, replacing the old Travis CI job - Fix Database::getHeaderInfo() signed-shift UB and use fixed-width types for the Header struct (#558) - Fix Savepoint destructor to catch all exceptions and track rollback state to avoid std::terminate (#559) +- Fix Transaction destructor to catch all exceptions to avoid std::terminate (#559) diff --git a/src/Transaction.cpp b/src/Transaction.cpp index 0e59e5f3..f31ef5e4 100644 --- a/src/Transaction.cpp +++ b/src/Transaction.cpp @@ -55,7 +55,7 @@ Transaction::~Transaction() { mDatabase.exec("ROLLBACK TRANSACTION"); } - catch (SQLite::Exception&) + catch (...) { // Never throw an exception in a destructor: error if already rollbacked, but no harm is caused by this. }