Hi,
Today I debugged an application that sometimes gave an error like this,
after several hours of stress testing:
ERROR: prepared statement "_PLAN000000000EA64DF0" already exists
The application is multi-threaded with a built-in connection pool. It
turned out to be a race condition between SQLFreeStmt and using the same
connection for another query.
SQLFreeStmt calls SC_Destructor, which calls SC_set_prepared(stmt,
FALSE). That is supposed to free the prepared statement in the backend,
by issuing a DEALLOCATE. Before that, however, it checks the status of
the connection; there's no point sending a DEALLOCATE if the connection
is dead already, or within an aborted transaction.
However, it's possible that the connection is in CONN_EXECUTING state,
if another thread is executing a different query using the same
connection. SC_set_prepared treats that the same as a dead connection,
and skips the DEALLOCATE. That leaks the prepared statement. Later, when
a new statement object is allocated to the same address, on the same
connection, and prepare a new statement using that handle, it will clash
with the leaked prepared statement in the backend, and give the
"prepared statement XXX already exists" error.
The fix is pretty straightforward: SQLFreeStmt (and SQLFreeHandle, which
is equivalent), needs to use a critical section like most of the API
functions do.
This particular problem only happens when dropping the statement
altogether with SQL_DROP, but I think SQL_CLOSE, SQL_RESET_PARAMS and
SQL_UNBIND also needs to be protected by a lock. Otherwise another
thread might be concurrently modifying the statement's data structures.
SQL_DROP will free the statement object, so that needs to be protected
by the connection's critical section rather than the statement's. Also,
the statement's critical section wouldn't be enough to stop other
threads from using the connection at the same time, so we'd still have
the same problem in SC_set_prepared.
So long story short, I'm going to apply the attached patch, which adds
locking to SQLFreeStmt and SQLFreeHandle. Attached is also a test
program to demonstrate the issue, using pthreads. I'm not going to add
this to the regression suite, though, because it would add a dependency
to pthreads, and it seems unlikely for this exact same bug to reappear
anyway.
Anyone see a problem with that?
PS. There are a few other API functions that are not protected by a
critical section: SQLCopyDesc, SQLGetDescField, SQLGetDiagField,
SQLGetDiagRec, SQLSetDescField. I'm not motivated enough to go fix them,
but I think at least some of them also have race conditions.
- Heikki
