Skip to content

Commit 6f01581

Browse files
TimelordUKclaude
andcommitted
fix: prevent race between ReleaseWorker and CloseWorker on statement factory
Close() and RemoveStatement() used different mutexes (_connectionMutex vs _statementMutex) allowing Close() to destroy _statementFactory while RemoveStatement() was still calling statements_.erase() on it, causing a read access violation crash on the map's internal tree. Fix: Close() now acquires _statementMutex before resetting the factory, and all methods accessing the factory take a local shared_ptr copy with null-check for defense in depth. Add stress test utility (samples/javascript/stress.js) with proc, connect, and mixed modes for soak-testing connection/statement lifecycle integrity. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent e4a4a55 commit 6f01581

2 files changed

Lines changed: 370 additions & 7 deletions

File tree

cpp/src/odbc/odbc_connection.cpp

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,14 @@ bool OdbcConnection::Close() {
130130
_connectionHandles.reset();
131131
_connectionHandles = nullptr;
132132
}
133-
_statementFactory.reset();
133+
{
134+
// Acquire _statementMutex before destroying the factory to prevent
135+
// racing with RemoveStatement() which also holds this lock while
136+
// accessing _statementFactory. Without this, Close() can destroy
137+
// the factory's statements_ map while RemoveStatement() is iterating it.
138+
std::lock_guard statementLock(_statementMutex);
139+
_statementFactory.reset();
140+
}
134141
_transactionManager.reset();
135142
_errorHandler.reset();
136143
connectionState = ConnectionClosed;
@@ -223,9 +230,18 @@ std::shared_ptr<IOdbcStatement> OdbcConnection::CreateStatement(
223230

224231
bool OdbcConnection::RemoveStatement(const std::shared_ptr<OdbcStatement>& statement) {
225232
std::lock_guard lock(_statementMutex);
233+
234+
// Take a local copy of the shared_ptr so the factory stays alive even if
235+
// Close() concurrently resets _statementFactory while we are working.
236+
auto factory = _statementFactory;
237+
if (!factory) {
238+
SQL_LOG_WARNING("RemoveStatement - factory already destroyed (connection closed)");
239+
return false;
240+
}
241+
226242
// Ask the factory to remove the statement
227243
if (statement) {
228-
_statementFactory->RemoveStatement(statement->GetStatementHandle().getStatementId());
244+
factory->RemoveStatement(statement->GetStatementHandle().getStatementId());
229245
}
230246

231247
return true;
@@ -234,16 +250,24 @@ bool OdbcConnection::RemoveStatement(const std::shared_ptr<OdbcStatement>& state
234250
bool OdbcConnection::RemoveStatement(int statementId) {
235251
std::lock_guard lock(_statementMutex);
236252

253+
// Take a local copy of the shared_ptr so the factory stays alive even if
254+
// Close() concurrently resets _statementFactory while we are working.
255+
auto factory = _statementFactory;
256+
if (!factory) {
257+
SQL_LOG_WARNING_STREAM("RemoveStatement - factory already destroyed for ID = " << statementId);
258+
return false;
259+
}
260+
237261
// First get the statement from the factory
238-
auto statement = _statementFactory->GetStatement(statementId);
262+
auto statement = factory->GetStatement(statementId);
239263
if (statement) {
240264
// Set CLOSED state before destroying statement (while JS context is still valid)
241265
statement->Close();
242266
}
243267

244268
SQL_LOG_DEBUG_STREAM("RemoveStatement ID = " << statementId);
245269
// Ask the factory to remove the statement
246-
_statementFactory->RemoveStatement(statementId);
270+
factory->RemoveStatement(statementId);
247271
return true;
248272
}
249273

@@ -261,7 +285,12 @@ bool OdbcConnection::CancelStatement(int queryId) {
261285
}
262286

263287
const auto statementId = it->second.getStatementId();
264-
auto statement = _statementFactory->GetStatement(statementId);
288+
auto factory = _statementFactory;
289+
if (!factory) {
290+
SQL_LOG_WARNING_STREAM("OdbcConnection::CancelStatement - factory already destroyed for ID = " << statementId);
291+
return false;
292+
}
293+
auto statement = factory->GetStatement(statementId);
265294
if (statement) {
266295
SQL_LOG_DEBUG_STREAM("OdbcConnection::Found statement for ID = " << statementId);
267296

@@ -289,7 +318,12 @@ std::shared_ptr<BoundDatumSet> OdbcConnection::UnbindStatement(int queryId) {
289318
}
290319

291320
const auto statementId = it->second.getStatementId();
292-
auto statement = _statementFactory->GetStatement(statementId);
321+
auto factory = _statementFactory;
322+
if (!factory) {
323+
SQL_LOG_WARNING_STREAM("OdbcConnection::UnbindStatement - factory already destroyed for ID = " << statementId);
324+
return nullptr;
325+
}
326+
auto statement = factory->GetStatement(statementId);
293327
if (statement) {
294328
SQL_LOG_DEBUG_STREAM("OdbcConnection::Found statement for ID = " << statementId);
295329

@@ -396,7 +430,12 @@ const std::vector<std::shared_ptr<OdbcError>>& OdbcConnection::GetErrors() const
396430
}
397431

398432
std::shared_ptr<IOdbcStatement> OdbcConnection::GetStatement(int statementId) const {
399-
return _statementFactory->GetStatement(statementId);
433+
auto factory = _statementFactory;
434+
if (!factory) {
435+
SQL_LOG_WARNING_STREAM("GetStatement - factory already destroyed for ID = " << statementId);
436+
return nullptr;
437+
}
438+
return factory->GetStatement(statementId);
400439
}
401440

402441
bool OdbcConnection::try_open(const std::u16string& connection_string, const int timeout) {

0 commit comments

Comments
 (0)