Skip to content

Data/SQLite: lock-free reads in SQLiteStatementImpl race with concurrent ATTACH #5375

Description

@aleks-f

Summary

Poco::Data::SQLite::SQLiteStatementImpl calls sqlite3_stmt_readonly and
sqlite3_changes without holding sqlite3_db_mutex. SQLite does not acquire
the connection mutex in their implementations:

// sqlite3.c
int sqlite3_stmt_readonly(sqlite3_stmt *pStmt){
  return pStmt ? ((Vdbe*)pStmt)->readOnly : 1;
}

sqlite3_int64 sqlite3_changes64(sqlite3 *db){
  ...
  return db->nChange;
}

Both are bare reads of struct fields that other threads are allowed to write
under the connection mutex. The canonical writer is
sqlite3ExpirePreparedStatements, called from ATTACH / DETACH /
schema-affecting DDL, which sets Vdbe::expired in the same machine word as
Vdbe::readOnly.

When one Poco::Data::Session is shared across threads and one thread does
ATTACH/DETACH while another is in hasNext, TSan reports a data race on
the Vdbe bytes (stack trace points at sqlite3.c:93062 in
sqlite3ExpirePreparedStatements).

How is POCO SQLite wrapper affected

The bare reads are inside SQLiteStatementImpl::hasNext (called on every
stepped row) and SQLiteStatementImpl::affectedRowCount. SQLite has had this
hole since shared cache was added; patching it upstream means changing every
call site of these accessors in every consumer. The minimal fix is to wrap
POCO's calls in Utility::SQLiteMutex (which is just
sqlite3_mutex_enter(sqlite3_db_mutex(db)) plus RAII release), bringing them
back under the same lock that sqlite3_step already takes internally.

Reproduction

Single Session shared between two threads; one runs SELECT in a loop, the
other runs ATTACH ':memory:' AS aux; DETACH aux; in a loop. Without the
wrap, TSan fires within milliseconds:

WARNING: ThreadSanitizer: data race
  Write of size 2 by thread T12:
    sqlite3ExpirePreparedStatements  sqlite3.c:93062
    sqlite3VdbeExec                  sqlite3.c:104659
    sqlite3_step                     sqlite3.c:94454
  Previous read of size 2 by thread T8:
    sqlite3_stmt_readonly            sqlite3.c:95554
    SQLiteStatementImpl::hasNext     SQLiteStatementImpl.cpp:241

Fix

Data/SQLite/src/SQLiteStatementImpl.cpp — wrap the bare reads in both
hasNext and affectedRowCount in Utility::SQLiteMutex. The wrap takes the
same sqlite3_db_mutex that sqlite3_step takes internally, so the read is
now atomic against any other thread holding that mutex (which is what the
ATTACH/DETACH writer side does).

In release mode the race is benign on real hardware: readOnly is set once at
prepare time and the torn-byte reads return a value that's still valid for the
statement's lifetime. The fix is for TSan correctness and for the C/C++ memory
model.

Why it stayed hidden for years

POCO Data/SQLite predates TSan. The standard usage pattern is one Session
per thread (or SessionPool), which never shares a connection across threads
and so never opens the race window. The hole only fires when a single
Session is used by multiple threads AND one of them runs an ATTACH/DETACH or
schema-affecting DDL; that is the exact pattern the newly introduced class Poco::Data::SQLite::MemoryDB uses.

Metadata

Metadata

Assignees

Labels

Type

Fields

No fields configured for Bug.

Projects

No projects

Relationships

None yet

Development

No branches or pull requests

Issue actions