Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Bug 1533616 - improve sdb_GetAttributeValueNoLock so it only executes…
… one sqlite statement per call r=jcj

Summary:
Prior to this change, sdb_GetAttributeValueNoLock would execute n sqlite
queries, where n is the number of elements in its input array.
As part of this change, sftk_updateMacs and sftk_updateEncrypted had to be
updated to operate on one attribute at a time (note that this does not regress
the performance of those functions because the code was already running two
sqlite queries per attribute - this just means this change didn't improve the
performance of those functions). Previously, these functions would try to obtain
an array of various attributes for the given object id. If no such attribute was
found, they would ignore these errors and just expect
sdb_GetAttributeValueNoLock to have properly populated its output data
regardless. The solution is to perform the same operation on each attribute one
at a time, and continue to the next if a given attribute isn't found.

Test Plan: https://treeherder.mozilla.org/#/jobs?repo=nss-try&revision=1b44b7588cb9a6806701394c08bf64d13f84b982

Reviewers: jcj

Reviewed By: jcj

Bug #: 1533616

Differential Revision: https://phabricator.services.mozilla.com/D23389

--HG--
extra : transplant_source : %D8D-%96%B5%21%8D%60%A4s8%C8%9E%11%E1g%EFZc%0E
extra : histedit_source : 9e229b0a06356d2072e34de18b9fc769cfd42bab
  • Loading branch information
mozkeeler committed Mar 20, 2019
1 parent 4201b8a commit f572a15
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 196 deletions.
98 changes: 57 additions & 41 deletions lib/softoken/sdb.c
Expand Up @@ -858,72 +858,88 @@ sdb_FindObjectsFinal(SDB *sdb, SDBFind *sdbFind)
return sdb_mapSQLError(sdb_p->type, sqlerr);
}

static const char GET_ATTRIBUTE_CMD[] = "SELECT ALL %s FROM %s WHERE id=$ID;";
CK_RV
sdb_GetAttributeValueNoLock(SDB *sdb, CK_OBJECT_HANDLE object_id,
CK_ATTRIBUTE *template, CK_ULONG count)
{
SDBPrivate *sdb_p = sdb->private;
sqlite3 *sqlDB = NULL;
sqlite3_stmt *stmt = NULL;
char *getStr = NULL;
char *newStr = NULL;
const char *table = NULL;
int sqlerr = SQLITE_OK;
CK_RV error = CKR_OK;
int found = 0;
int retry = 0;
unsigned int i;

if (count == 0) {
error = CKR_OBJECT_HANDLE_INVALID;
goto loser;
}

/* open a new db if necessary */
error = sdb_openDBLocal(sdb_p, &sqlDB, &table);
if (error != CKR_OK) {
goto loser;
}

char *columns = NULL;
for (i = 0; i < count; i++) {
getStr = sqlite3_mprintf("a%x", template[i].type);

if (getStr == NULL) {
error = CKR_HOST_MEMORY;
goto loser;
char *newColumns;
if (columns) {
newColumns = sqlite3_mprintf("%s, a%x", columns, template[i].type);
sqlite3_free(columns);
columns = NULL;
} else {
newColumns = sqlite3_mprintf("a%x", template[i].type);
}

newStr = sqlite3_mprintf(GET_ATTRIBUTE_CMD, getStr, table);
sqlite3_free(getStr);
getStr = NULL;
if (newStr == NULL) {
if (!newColumns) {
error = CKR_HOST_MEMORY;
goto loser;
}
columns = newColumns;
}
if (!columns) {
error = CKR_OBJECT_HANDLE_INVALID;
goto loser;
}

sqlerr = sqlite3_prepare_v2(sqlDB, newStr, -1, &stmt, NULL);
sqlite3_free(newStr);
newStr = NULL;
if (sqlerr == SQLITE_ERROR) {
template[i].ulValueLen = -1;
error = CKR_ATTRIBUTE_TYPE_INVALID;
continue;
} else if (sqlerr != SQLITE_OK) {
goto loser;
}
char *statement = sqlite3_mprintf("SELECT DISTINCT %s FROM %s where id=$ID LIMIT 1;",
columns, table);
sqlite3_free(columns);
columns = NULL;
if (!statement) {
error = CKR_HOST_MEMORY;
goto loser;
}

sqlerr = sqlite3_bind_int(stmt, 1, object_id);
if (sqlerr != SQLITE_OK) {
goto loser;
}
sqlerr = sqlite3_prepare_v2(sqlDB, statement, -1, &stmt, NULL);
sqlite3_free(statement);
statement = NULL;
if (sqlerr != SQLITE_OK) {
goto loser;
}

do {
sqlerr = sqlite3_step(stmt);
if (sqlerr == SQLITE_BUSY) {
PR_Sleep(SDB_BUSY_RETRY_TIME);
}
if (sqlerr == SQLITE_ROW) {
// NB: indices in sqlite3_bind_int are 1-indexed
sqlerr = sqlite3_bind_int(stmt, 1, object_id);
if (sqlerr != SQLITE_OK) {
goto loser;
}

do {
sqlerr = sqlite3_step(stmt);
if (sqlerr == SQLITE_BUSY) {
PR_Sleep(SDB_BUSY_RETRY_TIME);
}
if (sqlerr == SQLITE_ROW) {
PORT_Assert(!found);
for (i = 0; i < count; i++) {
unsigned int blobSize;
const char *blobData;

blobSize = sqlite3_column_bytes(stmt, 0);
blobData = sqlite3_column_blob(stmt, 0);
// NB: indices in sqlite_column_{bytes,blob} are 0-indexed
blobSize = sqlite3_column_bytes(stmt, i);
blobData = sqlite3_column_blob(stmt, i);
if (blobData == NULL) {
template[i].ulValueLen = -1;
error = CKR_ATTRIBUTE_TYPE_INVALID;
Expand All @@ -945,13 +961,13 @@ sdb_GetAttributeValueNoLock(SDB *sdb, CK_OBJECT_HANDLE object_id,
PORT_Memcpy(template[i].pValue, blobData, blobSize);
}
template[i].ulValueLen = blobSize;
found = 1;
}
} while (!sdb_done(sqlerr, &retry));
sqlite3_reset(stmt);
sqlite3_finalize(stmt);
stmt = NULL;
}
found = 1;
}
} while (!sdb_done(sqlerr, &retry));
sqlite3_reset(stmt);
sqlite3_finalize(stmt);
stmt = NULL;

loser:
/* fix up the error if necessary */
Expand Down

0 comments on commit f572a15

Please sign in to comment.