Rewrote querying in client prefs to not use prepared queries (bug 2698, r=pred).

Prepared queries had annoying race conditions and became unusable on connection timeouts.
This commit is contained in:
David Anderson 2008-10-20 03:40:46 -05:00
parent c0c9fc3267
commit 253109d1c9
5 changed files with 176 additions and 277 deletions

View File

@ -263,19 +263,19 @@ void CookieManager::OnClientDisconnecting(int client)
void CookieManager::ClientConnectCallback(int client, IQuery *data) void CookieManager::ClientConnectCallback(int client, IQuery *data)
{ {
IResultSet *results = data->GetResultSet(); IResultSet *results;
if (results == NULL)
if (data == NULL || (results = data->GetResultSet()) == NULL)
{ {
return; return;
} }
IResultRow *row;
do do
{ {
IResultRow *row = results->FetchRow(); if ((row = results->FetchRow()) == NULL)
if (row == NULL)
{ {
continue; break;
} }
const char *name; const char *name;
@ -307,7 +307,7 @@ void CookieManager::ClientConnectCallback(int client, IQuery *data)
clientData[client].push_back(pData); clientData[client].push_back(pData);
} while (results->MoreRows()); } while (results->MoreRows());
statsLoaded[client] = true; statsLoaded[client] = true;
@ -331,8 +331,9 @@ void CookieManager::InsertCookieCallback(Cookie *pCookie, int dbId)
void CookieManager::SelectIdCallback(Cookie *pCookie, IQuery *data) void CookieManager::SelectIdCallback(Cookie *pCookie, IQuery *data)
{ {
IResultSet *results = data->GetResultSet(); IResultSet *results;
if (results == NULL)
if (data == NULL || (results = data->GetResultSet()) == NULL)
{ {
return; return;
} }
@ -400,3 +401,4 @@ void CookieManager::OnPluginDestroyed(IPlugin *plugin)
} }
} }
} }

View File

@ -45,9 +45,7 @@ CookieTypeHandler g_CookieTypeHandler;
HandleType_t g_CookieIterator = 0; HandleType_t g_CookieIterator = 0;
CookieIteratorHandler g_CookieIteratorHandler; CookieIteratorHandler g_CookieIteratorHandler;
DbDriver g_DriverType;
int driver = 0;
bool ClientPrefs::SDK_OnLoad(char *error, size_t maxlength, bool late) bool ClientPrefs::SDK_OnLoad(char *error, size_t maxlength, bool late)
{ {
@ -141,16 +139,6 @@ void ClientPrefs::NotifyInterfaceDrop(SMInterface *pInterface)
{ {
if (Database != NULL && (void *)pInterface == (void *)(Database->GetDriver())) if (Database != NULL && (void *)pInterface == (void *)(Database->GetDriver()))
{ {
InsertCookieQuery->Destroy();
SelectDataQuery->Destroy();
SelectIdQuery->Destroy();
InsertDataQuery->Destroy();
InsertCookieQuery = NULL;
SelectDataQuery = NULL;
SelectIdQuery = NULL;
InsertDataQuery = NULL;
Database->Close(); Database->Close();
Database = NULL; Database = NULL;
} }
@ -177,28 +165,6 @@ void ClientPrefs::SDK_OnUnload()
plsys->RemovePluginsListener(&g_CookieManager); plsys->RemovePluginsListener(&g_CookieManager);
playerhelpers->RemoveClientListener(&g_CookieManager); playerhelpers->RemoveClientListener(&g_CookieManager);
/* Kill all our prepared queries - Queries are guaranteed to be flushed before this is called */
if (InsertCookieQuery != NULL)
{
InsertCookieQuery->Destroy();
}
if (SelectDataQuery != NULL)
{
SelectDataQuery->Destroy();
}
if (SelectIdQuery != NULL)
{
SelectIdQuery->Destroy();
}
if (InsertDataQuery != NULL)
{
InsertDataQuery->Destroy();
}
queryMutex->DestroyThis(); queryMutex->DestroyThis();
cookieMutex->DestroyThis(); cookieMutex->DestroyThis();
} }
@ -222,7 +188,7 @@ void ClientPrefs::DatabaseConnect()
if (strcmp(identifier, "sqlite") == 0) if (strcmp(identifier, "sqlite") == 0)
{ {
driver = DRIVER_SQLITE; g_DriverType = Driver_SQLite;
if (!Database->DoSimpleQuery( if (!Database->DoSimpleQuery(
"CREATE TABLE IF NOT EXISTS sm_cookies \ "CREATE TABLE IF NOT EXISTS sm_cookies \
@ -253,7 +219,7 @@ void ClientPrefs::DatabaseConnect()
} }
else if (strcmp(identifier, "mysql") == 0) else if (strcmp(identifier, "mysql") == 0)
{ {
driver = DRIVER_MYSQL; g_DriverType = Driver_MySQL;
if (!Database->DoSimpleQuery( if (!Database->DoSimpleQuery(
"CREATE TABLE IF NOT EXISTS sm_cookies \ "CREATE TABLE IF NOT EXISTS sm_cookies \
@ -289,82 +255,6 @@ void ClientPrefs::DatabaseConnect()
goto fatal_fail; goto fatal_fail;
} }
if (driver == DRIVER_MYSQL)
{
InsertCookieQuery = Database->PrepareQuery(
"INSERT IGNORE INTO sm_cookies(name, description, access) \
VALUES(?, ?, ?)",
error, sizeof(error), &errCode);
if (InsertCookieQuery == NULL)
{
g_pSM->LogMessage(myself, "Failed to prepare query InsertCookie: %s (%i)", error, errCode);
goto fatal_fail;
}
InsertDataQuery = Database->PrepareQuery(
"INSERT INTO sm_cookie_cache(player, cookie_id, value, timestamp) \
VALUES(?, ?, ?, ?) \
ON DUPLICATE KEY UPDATE value = ?, timestamp = ?",
error, sizeof(error), &errCode);
if (InsertDataQuery == NULL)
{
g_pSM->LogMessage(myself, "Failed to prepare query InsertData: %s (%i)", error, errCode);
goto fatal_fail;
}
}
else
{
InsertCookieQuery = Database->PrepareQuery(
"INSERT OR IGNORE INTO sm_cookies(name, description, access) \
VALUES(?, ?, ?)",
error, sizeof(error), &errCode);
if (InsertCookieQuery == NULL)
{
g_pSM->LogMessage(myself, "Failed to prepare query InsertCookie: %s (%i)", error, errCode);
goto fatal_fail;
}
InsertDataQuery = Database->PrepareQuery(
"INSERT OR REPLACE INTO sm_cookie_cache(player, cookie_id, value, timestamp) \
VALUES(?, ?, ?, ?)",
error, sizeof(error), &errCode);
if (InsertDataQuery == NULL)
{
g_pSM->LogMessage(myself, "Failed to prepare query InsertData: %s (%i)", error, errCode);
goto fatal_fail;
}
}
SelectDataQuery = Database->PrepareQuery(
"SELECT sm_cookies.name, sm_cookie_cache.value, sm_cookies.description, sm_cookies.access \
FROM sm_cookies \
JOIN sm_cookie_cache \
ON sm_cookies.id = sm_cookie_cache.cookie_id \
WHERE player = ?",
error, sizeof(error), &errCode);
if (SelectDataQuery == NULL)
{
g_pSM->LogMessage(myself, "Failed to prepare query SelectData: %s (%i)", error, errCode);
goto fatal_fail;
}
SelectIdQuery = Database->PrepareQuery(
"SELECT id \
FROM sm_cookies \
WHERE name=?",
error, sizeof(error), &errCode);
if (SelectIdQuery == NULL)
{
g_pSM->LogMessage(myself, "Failed to prepare query SelectId: %s (%i)", error, errCode);
goto fatal_fail;
}
databaseLoading = false; databaseLoading = false;
ProcessQueryCache(); ProcessQueryCache();
@ -372,26 +262,6 @@ void ClientPrefs::DatabaseConnect()
return; return;
fatal_fail: fatal_fail:
if (SelectIdQuery != NULL)
{
SelectIdQuery->Destroy();
SelectIdQuery = NULL;
}
if (SelectDataQuery != NULL)
{
SelectDataQuery->Destroy();
SelectDataQuery = NULL;
}
if (InsertCookieQuery != NULL)
{
InsertCookieQuery->Destroy();
InsertCookieQuery = NULL;
}
if (InsertDataQuery != NULL)
{
InsertDataQuery->Destroy();
InsertDataQuery = NULL;
}
Database->Close(); Database->Close();
Database = NULL; Database = NULL;
databaseLoading = false; databaseLoading = false;
@ -414,19 +284,7 @@ bool ClientPrefs::AddQueryToQueue( TQueryOp *query )
if (Database) if (Database)
{ {
query->SetDatabase(Database); query->SetDatabase(Database);
query->SetPreparedQuery(); dbi->AddToThreadQueue(query, PrioQueue_Normal);
if (query->GetQuery() == NULL && query->GetType() != Query_Connect)
{
g_pSM->LogError(myself, "Invalid aqtq query %d being inserted! glorp: %p, %p",
query->GetType(),
query->GetDriver(),
query);
query->Destroy();
}
else
{
dbi->AddToThreadQueue(query, PrioQueue_Normal);
}
return true; return true;
} }
@ -451,19 +309,7 @@ void ClientPrefs::ProcessQueryCache()
if (Database != NULL) if (Database != NULL)
{ {
op->SetDatabase(Database); op->SetDatabase(Database);
op->SetPreparedQuery(); dbi->AddToThreadQueue(op, PrioQueue_Normal);
if (op->GetQuery() == NULL && op->GetType() != Query_Connect)
{
g_pSM->LogError(myself, "Invalid pqc query %d being inserted! glorp: %p, %p",
op->GetType(),
op->GetDriver(),
op);
op->Destroy();
}
else
{
dbi->AddToThreadQueue(op, PrioQueue_Normal);
}
} }
else else
{ {

View File

@ -42,8 +42,11 @@
#include "menus.h" #include "menus.h"
#include "query.h" #include "query.h"
#define DRIVER_MYSQL 1 enum DbDriver
#define DRIVER_SQLITE 0 {
Driver_MySQL,
Driver_SQLite
};
#define MAX_TRANSLATE_PARAMS 32 #define MAX_TRANSLATE_PARAMS 32
@ -139,11 +142,6 @@ public:
IPhraseCollection *phrases; IPhraseCollection *phrases;
const DatabaseInfo *DBInfo; const DatabaseInfo *DBInfo;
IPreparedQuery *InsertCookieQuery;
IPreparedQuery *SelectDataQuery;
IPreparedQuery *InsertDataQuery;
IPreparedQuery *SelectIdQuery;
IMutex *cookieMutex; IMutex *cookieMutex;
private: private:
@ -187,6 +185,7 @@ bool Translate(char *buffer,
size_t *pOutLength, size_t *pOutLength,
...); ...);
extern int driver; extern DbDriver g_DriverType;
#endif // _INCLUDE_SOURCEMOD_EXTENSION_PROPER_H_ #endif // _INCLUDE_SOURCEMOD_EXTENSION_PROPER_H_

View File

@ -41,32 +41,29 @@ void TQueryOp::RunThinkPart()
return; return;
} }
if (m_pQuery) switch (m_type)
{ {
switch (m_type) case Query_InsertCookie:
{ {
case Query_InsertCookie: g_CookieManager.InsertCookieCallback(m_pCookie, m_insertId);
{ break;
g_CookieManager.InsertCookieCallback(m_pCookie, m_insertId); }
break;
}
case Query_SelectData: case Query_SelectData:
{ {
g_CookieManager.ClientConnectCallback(m_client, m_pQuery); g_CookieManager.ClientConnectCallback(m_client, m_pResult);
break; break;
} }
case Query_SelectId: case Query_SelectId:
{ {
g_CookieManager.SelectIdCallback(m_pCookie, m_pQuery); g_CookieManager.SelectIdCallback(m_pCookie, m_pResult);
break; break;
} }
default: default:
{ {
break; break;
}
} }
} }
} }
@ -81,19 +78,18 @@ void TQueryOp::RunThreadPart()
{ {
assert(m_database != NULL); assert(m_database != NULL);
/* I don't think this is needed anymore... keeping for now. */
m_database->LockForFullAtomicOperation(); m_database->LockForFullAtomicOperation();
if (!BindParamsAndRun()) if (!BindParamsAndRun())
{ {
g_pSM->LogError(myself, g_pSM->LogError(myself,
"Failed SQL Query, Error: \"%s\" (Query id %i - client %i)", "Failed SQL Query, Error: \"%s\" (Query id %i - client %i)",
m_pQuery ? m_pQuery->GetError() : "NULL QUERY", m_database->GetError(),
m_type, m_type,
m_client); m_client);
} }
m_insertId = m_database->GetInsertID();
m_database->UnlockFromFullAtomicOperation(); m_database->UnlockFromFullAtomicOperation();
} }
} }
@ -104,6 +100,7 @@ IDBDriver *TQueryOp::GetDriver()
return m_database->GetDriver(); return m_database->GetDriver();
} }
IdentityToken_t *TQueryOp::GetOwner() IdentityToken_t *TQueryOp::GetOwner()
{ {
return myself->GetIdentity(); return myself->GetIdentity();
@ -111,6 +108,10 @@ IdentityToken_t *TQueryOp::GetOwner()
void TQueryOp::Destroy() void TQueryOp::Destroy()
{ {
if (m_pResult != NULL)
{
m_pResult->Destroy();
}
delete this; delete this;
} }
@ -118,19 +119,21 @@ TQueryOp::TQueryOp(enum querytype type, int client)
{ {
m_type = type; m_type = type;
m_client = client; m_client = client;
m_pQuery = NULL;
m_database = NULL; m_database = NULL;
m_insertId = -1;
m_pResult = NULL;
} }
TQueryOp::TQueryOp(enum querytype type, Cookie *cookie) TQueryOp::TQueryOp(enum querytype type, Cookie *cookie)
{ {
m_type = type; m_type = type;
m_pCookie = cookie; m_pCookie = cookie;
m_pQuery = NULL;
m_database = NULL; m_database = NULL;
m_insertId = -1;
m_pResult = NULL;
} }
void TQueryOp::SetDatabase( IDatabase *db ) void TQueryOp::SetDatabase(IDatabase *db)
{ {
m_database = db; m_database = db;
m_database->IncReferenceCount(); m_database->IncReferenceCount();
@ -138,100 +141,160 @@ void TQueryOp::SetDatabase( IDatabase *db )
bool TQueryOp::BindParamsAndRun() bool TQueryOp::BindParamsAndRun()
{ {
assert(m_pQuery != NULL); size_t ignore;
char query[2048];
/* :TODO: remove this check. */
if (m_pQuery == NULL)
{
g_pSM->LogError(myself, "Attempted to run with a NULL Query (type %d)", m_type);
return false;
}
switch (m_type) switch (m_type)
{ {
case Query_InsertCookie: case Query_InsertCookie:
{ {
m_pQuery->BindParamString(0, m_params.cookie->name, false); char safe_name[MAX_NAME_LENGTH*2 + 1];
m_pQuery->BindParamString(1, m_params.cookie->description, false); char safe_desc[MAX_DESC_LENGTH*2 + 1];
m_pQuery->BindParamInt(2, m_params.cookie->access);
break; m_database->QuoteString(m_params.cookie->name,
} safe_name,
sizeof(safe_name),
&ignore);
m_database->QuoteString(m_params.cookie->description,
safe_desc,
sizeof(safe_desc),
&ignore);
case Query_SelectData: if (g_DriverType == Driver_MySQL)
{
m_pQuery->BindParamString(0, m_params.steamId, false);
break;
}
case Query_InsertData:
{
m_pQuery->BindParamString(0, m_params.steamId, false);
m_pQuery->BindParamInt(1, m_params.cookieId);
m_pQuery->BindParamString(2, m_params.data->value, false);
m_pQuery->BindParamInt(3, (unsigned int)time(NULL), false);
if (driver == DRIVER_MYSQL)
{ {
m_pQuery->BindParamString(4, m_params.data->value, false); UTIL_Format(query,
m_pQuery->BindParamInt(5, (unsigned int)time(NULL), false); sizeof(query),
"INSERT IGNORE INTO sm_cookies (name, description, access) \
VALUES (\"%s\", \"%s\", %d)",
safe_name,
safe_desc,
m_params.cookie->access);
}
else if (g_DriverType == Driver_SQLite)
{
UTIL_Format(query,
sizeof(query),
"INSERT OR IGNORE INTO sm_cookies (name, description, access) \
VALUES (\"%s\", \"%s\", %d)",
safe_name,
safe_desc,
m_params.cookie->access);
} }
break; if (!m_database->DoSimpleQuery(query))
} {
return false;
}
case Query_SelectId: m_insertId = m_database->GetInsertID();
{
/* the steamId var was actually used to store the name of the cookie - Save duplicating vars */
m_pQuery->BindParamString(0, m_params.steamId, false);
break; return true;
}
default:
{
break;
}
}
return m_pQuery->Execute();
}
void TQueryOp::SetPreparedQuery()
{
switch (m_type)
{
case Query_InsertCookie:
{
m_pQuery = g_ClientPrefs.InsertCookieQuery;
break;
} }
case Query_SelectData: case Query_SelectData:
{ {
m_pQuery = g_ClientPrefs.SelectDataQuery; char safe_str[128];
break;
m_database->QuoteString(m_params.steamId, safe_str, sizeof(safe_str), &ignore);
UTIL_Format(query,
sizeof(query),
"SELECT sm_cookies.name, sm_cookie_cache.value, sm_cookies.description, \
sm_cookies.access \
FROM sm_cookies \
JOIN sm_cookie_cache \
ON sm_cookies.id = sm_cookie_cache.cookie_id \
WHERE player = \"%s\"",
safe_str);
m_pResult = m_database->DoQuery(query);
return (m_pResult != NULL);
} }
case Query_InsertData: case Query_InsertData:
{ {
m_pQuery = g_ClientPrefs.InsertDataQuery; time_t t = time(NULL);
break; char safe_id[128];
char safe_val[MAX_VALUE_LENGTH*2 + 1];
m_database->QuoteString(m_params.steamId,
safe_id,
sizeof(safe_id),
&ignore);
m_database->QuoteString(m_params.data->value,
safe_val,
sizeof(safe_val),
&ignore);
if (g_DriverType == Driver_MySQL)
{
UTIL_Format(query,
sizeof(query),
"INSERT INTO sm_cookie_cache (player, cookie_id, value, timestamp) \
VALUES (\"%s\", %d, \"%s\", %d) \
ON DUPLICATE KEY UPDATE \
value = \"%s\", timestamp = %d",
safe_id,
m_params.cookieId,
safe_val,
(unsigned int)t,
safe_val,
(unsigned int)t);
}
else if (g_DriverType == Driver_SQLite)
{
UTIL_Format(query,
sizeof(query),
"INSERT OR REPLACE INTO sm_cookie_cache \
(player, cookie_id, value, timestamp) \
VALUES (\"%s\", %d, \"%s\", %d)",
safe_id,
m_params.cookieId,
safe_val,
(unsigned int)t);
}
if (!m_database->DoSimpleQuery(query))
{
return false;
}
m_insertId = m_database->GetInsertID();
return true;
} }
case Query_SelectId: case Query_SelectId:
{ {
m_pQuery = g_ClientPrefs.SelectIdQuery; char safe_name[MAX_NAME_LENGTH*2 + 1];
break;
/* the steamId var was actually used to store the name of the cookie - Save duplicating vars */
m_database->QuoteString(m_params.steamId,
safe_name,
sizeof(safe_name),
&ignore);
UTIL_Format(query,
sizeof(query),
"SELECT id FROM sm_cookies WHERE name = \"%s\"",
safe_name);
m_pResult = m_database->DoQuery(query);
return (m_pResult != NULL);
} }
default: default:
{ {
break; break;
} }
} }
assert(false);
return false;
} }
ParamData::~ParamData() ParamData::~ParamData()
@ -266,3 +329,4 @@ ParamData::ParamData()
steamId[0] = '\0'; steamId[0] = '\0';
cookieId = 0; cookieId = 0;
} }

View File

@ -45,7 +45,6 @@ enum querytype
Query_Connect, Query_Connect,
}; };
struct PreparedQueryWrapper;
struct Cookie; struct Cookie;
struct CookieData; struct CookieData;
#define MAX_NAME_LENGTH 30 #define MAX_NAME_LENGTH 30
@ -77,7 +76,6 @@ public:
IdentityToken_t *GetOwner(); IdentityToken_t *GetOwner();
void SetDatabase(IDatabase *db); void SetDatabase(IDatabase *db);
void SetPreparedQuery();
void Destroy(); void Destroy();
@ -91,24 +89,14 @@ public:
/* Params to be bound */ /* Params to be bound */
ParamData m_params; ParamData m_params;
inline IPreparedQuery *GetQuery()
{
return m_pQuery;
}
inline querytype GetType()
{
return m_type;
}
inline IDatabase *GetDB() inline IDatabase *GetDB()
{ {
return m_database; return m_database;
} }
private: private:
IPreparedQuery *m_pQuery;
IDatabase *m_database; IDatabase *m_database;
IQuery *m_pResult;
/* Query type */ /* Query type */
enum querytype m_type; enum querytype m_type;