From 25564c5ef5e5af3ca07de6c82fd107f91cb93b13 Mon Sep 17 00:00:00 2001 From: onefang Date: Thu, 14 May 2020 23:10:15 +1000 Subject: No more database leaks. --- src/sledjchisl/sledjchisl.c | 144 +++++++++++++++++++------------------------- 1 file changed, 61 insertions(+), 83 deletions(-) diff --git a/src/sledjchisl/sledjchisl.c b/src/sledjchisl/sledjchisl.c index 5a06fa2..ddb041d 100644 --- a/src/sledjchisl/sledjchisl.c +++ b/src/sledjchisl/sledjchisl.c @@ -979,7 +979,13 @@ static boolean dbCheckError(MYSQL *db, char *error, char *sql) return FALSE; } -typedef struct _dbField dbField; +typedef struct _dbFields dbFields; +struct _dbFields +{ + qlisttbl_t *flds; + int count; +}; +typedef struct _dbField dbField; struct _dbField { char *name; @@ -989,11 +995,11 @@ struct _dbField unsigned int decimals; }; -qlisttbl_t *dbGetFields(MYSQL *db, char *table) +dbFields *dbGetFields(MYSQL *db, char *table) { static qhashtbl_t *tables = NULL; if (NULL == tables) tables = qhashtbl(0, 0); - qlisttbl_t *ret = tables->get(tables, table, NULL, false); + dbFields *ret = tables->get(tables, table, NULL, false); if (NULL == ret) { @@ -1028,7 +1034,9 @@ d("Getting field metadata for %s", table); { unsigned int i, num_fields = mysql_num_fields(res); - ret = qlisttbl(QLISTTBL_UNIQUE | QLISTTBL_LOOKUPFORWARD); + ret = xmalloc(sizeof(dbFields)); + ret->flds = qlisttbl(QLISTTBL_UNIQUE | QLISTTBL_LOOKUPFORWARD); + ret->count = 1; for (i = 0; i < num_fields; i++) { dbField *fld = xmalloc(sizeof(dbField)); @@ -1037,7 +1045,7 @@ d("Getting field metadata for %s", table); fld->length = fields[i].length; fld->flags = fields[i].flags; fld->decimals = fields[i].decimals; - ret->put(ret, fld->name, fld, sizeof(*fld)); + ret->flds->put(ret->flds, fld->name, fld, sizeof(*fld)); free(fld); } tables->put(tables, table, ret, sizeof(*ret)); @@ -1047,24 +1055,30 @@ d("Getting field metadata for %s", table); } free(sql); } + else // Reference count these, coz some tables are used more than once. + ret->count++; return ret; } -void dbFreeFields(qlisttbl_t *flds) +void dbFreeFields(dbFields *flds) { - qlisttbl_obj_t obj; - memset((void *) &obj, 0, sizeof(obj)); - flds->lock(flds); -d("Freeing fields."); - while(flds->getnext(flds, &obj, NULL, false) == true) + flds->count--; + if (0 >= flds->count) { - dbField *fld = (dbField *) obj.data; -d("Freeing field %s", fld->name); - free(fld->name); + qlisttbl_obj_t obj; + + memset((void *) &obj, 0, sizeof(obj)); + flds->flds->lock(flds->flds); + while(flds->flds->getnext(flds->flds, &obj, NULL, false) == true) + { + dbField *fld = (dbField *) obj.data; + free(fld->name); + } + flds->flds->unlock(flds->flds); + flds->flds->free(flds->flds); + free(flds); } - flds->unlock(flds); - flds->free(flds); } enum dbCommandType @@ -1080,7 +1094,8 @@ struct _dbRequest { MYSQL *db; char *table, *join, *where, *order, *sql; - MYSQL_STMT *prep, *prep0; // NOTE - executing it stores state in this. + MYSQL_STMT *prep; // NOTE - executing it stores state in this. + dbFields *fields; qlisttbl_t *flds; int inCount, outCount, rowCount; char **inParams, **outParams; @@ -1111,13 +1126,14 @@ int dbDoSomething(dbRequest *req, boolean count, ...) if (0 == req->type) req->type = CT_SELECT; - req->flds = dbGetFields(req->db, req->table); - if (NULL == req->flds) + req->fields = dbGetFields(req->db, req->table); + if (NULL == req->fields) { E("Unknown fields for table %s.", req->table); ret++; goto end; } + req->flds = req->fields->flds; switch (req->type) { @@ -1200,8 +1216,6 @@ int dbDoSomething(dbRequest *req, boolean count, ...) d("New SQL statement - %s", req->sql); // prepare statement with the other fields req->prep = mysql_stmt_init(req->db); - // Save the pointer before something mysteriously clobbers it, so we can close it later. - req->prep0 = req->prep; if (NULL == req->prep) { E("Statement prepare init failed: %s\n", mysql_stmt_error(req->prep)); @@ -1820,17 +1834,6 @@ end: double n = (now.tv_sec * 1000000000.0) + now.tv_nsec; double t = (then.tv_sec * 1000000000.0) + then.tv_nsec; T("dbDoSomething(%s) took %lf seconds", req->sql, (n - t) / 1000000000.0); -/* - if (NULL != req->prep) - I("The prepared statement itself is NOT NULL."); - else - W("The prepared statement itself is NULL!"); - - if (NULL != req->prep0) - I("The prepared0 statement itself is NOT NULL."); - else - W("The prepared0 statement itself is NULL!"); -*/ return ret; } @@ -1863,30 +1866,20 @@ void dbFreeRequest(dbRequest *req) { int i; -// TODO - this leaks for some bizare reason. Not even req->sql survives. D("Cleaning up prepared database request %s - %s %d %d", req->table, req->where, req->outCount, req->inCount); - if (NULL != req->prep) - I("The prepared statement itself is NOT NULL."); - else - W("The prepared statement itself is NULL!"); - - if (NULL != req->prep0) - I("The prepared0 statement itself is NOT NULL."); - else - W("The prepared0 statement itself is NULL!"); - - if (NULL != req->flds) - dbFreeFields(req->flds); + if (NULL != req->fields) + { + dbFreeFields(req->fields); + req->fields = NULL; + } else - D("No fields to clean up for %s - %s.", req->table, req->where); + D(" No fields to clean up for %s - %s.", req->table, req->where); if (NULL != req->outBind) { -d("Free outBind"); for (i = 0; i < req->outCount; i++) { -d("Free outBind %d %s", i, req->sql); if (NULL != req->outBind[i].buffer) free(req->outBind[i].buffer); if (NULL != req->outBind[i].length) free(req->outBind[i].length); if (NULL != req->outBind[i].error) free(req->outBind[i].error); @@ -1895,13 +1888,11 @@ d("Free outBind %d %s", i, req->sql); free(req->outBind); } else - D("No out binds to clean up for %s - %s.", req->table, req->where); + D(" No out binds to clean up for %s - %s.", req->table, req->where); if (NULL != req->inBind) { -d("Free inBind"); for (i = 0; i < req->inCount; i++) { -d("Free inBind %d %s", i, req->sql); if (NULL != req->inBind[i].buffer) free(req->inBind[i].buffer); if (NULL != req->inBind[i].length) free(req->inBind[i].length); if (NULL != req->inBind[i].error) free(req->inBind[i].error); @@ -1910,24 +1901,20 @@ d("Free inBind %d %s", i, req->sql); free(req->inBind); } else - D("No in binds to clean up for %s - %s.", req->table, req->where); + D(" No in binds to clean up for %s - %s.", req->table, req->where); if (req->freeOutParams) free(req->outParams); else - D("No out params to clean up for %s - %s.", req->table, req->where); + D(" No out params to clean up for %s - %s.", req->table, req->where); if (NULL != req->sql) free(req->sql); else - D("No SQL to clean up for %s - %s.", req->table, req->where); - if (NULL != req->prep0) + D(" No SQL to clean up for %s - %s.", req->table, req->where); + if (NULL != req->prep) { -// TODO - this leaks for some bizare reason. - D(" Cleaning up the prepared statement."); - if (0 != mysql_stmt_close(req->prep0)) + if (0 != mysql_stmt_close(req->prep)) C(" Unable to close the prepared statement!"); - free(req->prep0); + req->prep = NULL; } - else - W(" The prepared statement itself is NULL!"); } my_ulonglong dbCount(MYSQL *db, char *table, char *where) @@ -2194,7 +2181,7 @@ gridStats *getStats(MYSQL *db, gridStats *stats) rgnSizes->inParams = szi; rgnSizes->outParams = szo; rgnSizes->where = "sizeX != 0"; - dbRequests->addfirst(dbRequests, rgnSizes, sizeof(*rgnSizes)); + dbRequests->addfirst(dbRequests, &rgnSizes, sizeof(dbRequest *)); } dbDoSomething(rgnSizes, FALSE); rowData *rows = rgnSizes->rows; @@ -3876,7 +3863,7 @@ notWritten: acntsI->outParams = szo; acntsI->where = ""; acntsI->type = CT_CREATE; - dbRequests->addfirst(dbRequests, acntsI, sizeof(*acntsI)); + dbRequests->addfirst(dbRequests, &acntsI, sizeof(dbRequest *)); } static dbRequest *authI = NULL; if (NULL == authI) @@ -3890,7 +3877,7 @@ notWritten: authI->outParams = szo; authI->where = ""; authI->type = CT_CREATE; - dbRequests->addfirst(dbRequests, authI, sizeof(*authI)); + dbRequests->addfirst(dbRequests, &authI, sizeof(dbRequest *)); } static dbRequest *invFolderI = NULL; if (NULL == invFolderI) @@ -3913,7 +3900,7 @@ notWritten: invFolderI->outParams = szo; invFolderI->where = ""; invFolderI->type = CT_CREATE; - dbRequests->addfirst(dbRequests, invFolderI, sizeof(*invFolderI)); + dbRequests->addfirst(dbRequests, &invFolderI, sizeof(dbRequest *)); } static dbRequest *gUserI = NULL; if (NULL == gUserI) @@ -3928,7 +3915,7 @@ notWritten: gUserI->outParams = szo; gUserI->where = ""; gUserI->type = CT_CREATE; - dbRequests->addfirst(dbRequests, gUserI, sizeof(*gUserI)); + dbRequests->addfirst(dbRequests, &gUserI, sizeof(dbRequest *)); } I("Creating database user %s %s.", uuid, getStrH(Rd->stuff, "name")); @@ -5116,7 +5103,7 @@ static int accountRead(reqData *Rd, char *uuid, char *firstName, char *lastName) uuids->inParams = szi; uuids->outParams = szo; uuids->where = "PrincipalID=?"; - dbRequests->addfirst(dbRequests, uuids, sizeof(*uuids)); + dbRequests->addfirst(dbRequests, &uuids, sizeof(dbRequest *)); } static dbRequest *acnts = NULL; if (NULL == acnts) @@ -5129,7 +5116,7 @@ static int accountRead(reqData *Rd, char *uuid, char *firstName, char *lastName) acnts->inParams = szi; acnts->outParams = szo; acnts->where = "FirstName=? and LastName=?"; - dbRequests->addfirst(dbRequests, acnts, sizeof(*acnts)); + dbRequests->addfirst(dbRequests, &acnts, sizeof(dbRequest *)); } static dbRequest *auth = NULL; if (NULL == auth) @@ -5142,7 +5129,7 @@ static int accountRead(reqData *Rd, char *uuid, char *firstName, char *lastName) auth->inParams = szi; auth->outParams = szo; auth->where = "UUID=?"; - dbRequests->addfirst(dbRequests, auth, sizeof(*auth)); + dbRequests->addfirst(dbRequests, &auth, sizeof(dbRequest *)); } Rd->fromDb = FALSE; @@ -6328,23 +6315,14 @@ void account_html(char *file, reqData *Rd, HTMLfile *thisFile) static void cleanup(void) { -// TODO - not sure why, but this gets called twice on quitting. +// TODO - not sure why, but this gets called twice on quitting sometimes. C("Caught signal, or quitting, cleaning up."); - dbRequest *req = NULL; + dbRequest **rq; - while (NULL != (req = (dbRequest *) dbRequests->getat(dbRequests, 0, NULL, false))) + while (NULL != (rq = (dbRequest **) dbRequests->popfirst(dbRequests, NULL))) { - if (NULL != req->prep) - I("The prepared statement itself is NOT NULL."); - else - W("The prepared statement itself is NULL!"); - - if (NULL != req->prep0) - I("The prepared0 statement itself is NOT NULL."); - else - W("The prepared0 statement itself is NULL!"); - dbFreeRequest(req); - dbRequests->removefirst(dbRequests); + dbFreeRequest(*rq); + free(rq); } if (accountPages) -- cgit v1.1