security-and-privacy/gnupg
security-and-privacy/gnupg
1
0
Fork 0
mirror of git://git.gnupg.org/gnupg.git synced 2025-06-17 20:27:03 +02:00
Code Activity

g10: Improve TOFU batch update code.

Browse Source 
* g10/gpg.h (tofu): Rename field batch_update_ref to
batch_updated_wanted.
* g10/tofu.c (struct tofu_dbs_s): Rename field batch_update to
in_batch_transaction.
(begin_transaction): Only end an extant batch transaction if we are
not in a normal transaction.  When ending a batch transaction, really
end it.  Update ctrl->tofu.batch_update_started when starting a batch
transaction.
(end_transaction): Only release a batch transaction if ONLY_BATCH is
true.  When releasing a batch transaction, assert that there is no
open normal transaction.  Only allow DBS to be NULL if ONLY_BATCH is
true.
(tofu_begin_batch_update): Don't update
ctrl->tofu.batch_update_started.
(opendbs): Call end_transaction unconditionally.

--
Signed-off-by: Neal H. Walfield <neal@g10code.com>
This commit is contained in:
Neal H. Walfield 2016-08-30 15:37:45 +02:00
parent 3beeaa70bd
commit 371ae66e9d
2 changed files with 64 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
g10/gpg.h
View File

@ -83,7 +83,7 @@ struct server_control_s
struct { struct {
tofu_dbs_t dbs; tofu_dbs_t dbs;
int in_transaction; int in_transaction;
int batch_update_ref; int batch_updated_wanted;
time_t batch_update_started; time_t batch_update_started;
} tofu; } tofu;

108
g10/tofu.c
View File

@ -81,7 +81,7 @@ struct tofu_dbs_s
sqlite3_stmt *register_insert; sqlite3_stmt *register_insert;
} s; } s;
int batch_update; int in_batch_transaction;
}; };
@ -169,26 +169,39 @@ begin_transaction (ctrl_t ctrl, int only_batch)
log_assert (dbs); log_assert (dbs);
if (ctrl->tofu.batch_update_ref /* If we've been in batch update mode for a while (on average, more
* than 500 ms), to prevent starving other gpg processes, we drop
* and retake the batch lock.
*
* Note: if we wanted higher resolution, we could use
* npth_clock_gettime. */
if (/* No real transactions. */
ctrl->tofu.in_transaction == 0
/* There is an open batch transaction. */
&& dbs->in_batch_transaction
/* And some time has gone by since it was started. */
&& ctrl->tofu.batch_update_started != gnupg_get_time ()) && ctrl->tofu.batch_update_started != gnupg_get_time ())
{ {
/* We've been in batch update mode for a while (on average, more /* If we are in a batch update, then batch updates better have
* than 500 ms). To prevent starving other gpg processes, we been enabled. */
* drop and retake the batch lock. log_assert (ctrl->tofu.batch_updated_wanted);
*
* Note: if we wanted higher resolution, we could use
* npth_clock_gettime. */
if (dbs->batch_update)
end_transaction (ctrl, 1);
ctrl->tofu.batch_update_started = gnupg_get_time (); end_transaction (ctrl, 2);
/* Yield to allow another process a chance to run. */ /* Yield to allow another process a chance to run. */
gpgrt_yield (); gpgrt_yield ();
} }
if (ctrl->tofu.batch_update_ref && !dbs->batch_update) if (/* Batch mode is enabled. */
ctrl->tofu.batch_updated_wanted
/* But we don't have an open batch transaction. */
&& !dbs->in_batch_transaction)
{ {
/* We are in batch mode, but we don't have an open batch
* transaction. Since the batch save point must be the outer
* save point, it must be taken before the inner save point. */
log_assert (ctrl->tofu.in_transaction == 0);
rc = gpgsql_stepx (dbs->db, &dbs->s.savepoint_batch, rc = gpgsql_stepx (dbs->db, &dbs->s.savepoint_batch,
NULL, NULL, &err, NULL, NULL, &err,
"savepoint batch;", SQLITE_ARG_END); "savepoint batch;", SQLITE_ARG_END);
@ -200,7 +213,8 @@ begin_transaction (ctrl_t ctrl, int only_batch)
return gpg_error (GPG_ERR_GENERAL); return gpg_error (GPG_ERR_GENERAL);
} }
dbs->batch_update = 1; dbs->in_batch_transaction = 1;
ctrl->tofu.batch_update_started = gnupg_get_time ();
} }
if (only_batch) if (only_batch)
@ -235,35 +249,44 @@ end_transaction (ctrl_t ctrl, int only_batch)
int rc; int rc;
char *err = NULL; char *err = NULL;
if (!dbs) if (only_batch)
return 0; /* Shortcut to allow for easier cleanup code. */
if ((!ctrl->tofu.batch_update_ref || only_batch == 2) && dbs->batch_update)
{ {
/* The batch transaction is still in open, but we left batch if (!dbs)
* mode. */ return 0; /* Shortcut to allow for easier cleanup code. */
dbs->batch_update = 0;
rc = gpgsql_stepx (dbs->db, &dbs->s.savepoint_batch_commit, /* If we are releasing the batch transaction, then we better not
NULL, NULL, &err, be in a normal transaction. */
"release batch;", SQLITE_ARG_END); log_assert (ctrl->tofu.in_transaction == 0);
if (rc)
if (/* Batch mode disabled? */
(!ctrl->tofu.batch_updated_wanted || only_batch == 2)
/* But, we still have an open batch transaction? */
&& dbs->in_batch_transaction)
{ {
log_error (_("error committing transaction on TOFU database: %s\n"), /* The batch transaction is still in open, but we've left
err); * batch mode. */
sqlite3_free (err); dbs->in_batch_transaction = 0;
return gpg_error (GPG_ERR_GENERAL);
rc = gpgsql_stepx (dbs->db, &dbs->s.savepoint_batch_commit,
NULL, NULL, &err,
"release batch;", SQLITE_ARG_END);
if (rc)
{
log_error (_("error committing transaction on TOFU database: %s\n"),
err);
sqlite3_free (err);
return gpg_error (GPG_ERR_GENERAL);
}
return 0;
} }
/* Releasing an outer transaction releases an open inner
transactions. We're done. */
return 0; return 0;
} }
if (only_batch) log_assert (dbs);
return 0;
log_assert (ctrl->tofu.in_transaction > 0); log_assert (ctrl->tofu.in_transaction > 0);
rc = gpgsql_exec_printf (dbs->db, NULL, NULL, &err, rc = gpgsql_exec_printf (dbs->db, NULL, NULL, &err,
"release inner%d;", ctrl->tofu.in_transaction); "release inner%d;", ctrl->tofu.in_transaction);
if (rc) if (rc)
@ -287,8 +310,7 @@ rollback_transaction (ctrl_t ctrl)
int rc; int rc;
char *err = NULL; char *err = NULL;
if (!dbs) log_assert (dbs);
return 0; /* Shortcut to allow for easier cleanup code. */
log_assert (ctrl->tofu.in_transaction > 0); log_assert (ctrl->tofu.in_transaction > 0);
/* Be careful to not any progress made by closed transactions in /* Be careful to not any progress made by closed transactions in
@ -313,19 +335,16 @@ rollback_transaction (ctrl_t ctrl)
void void
tofu_begin_batch_update (ctrl_t ctrl) tofu_begin_batch_update (ctrl_t ctrl)
{ {
if (!ctrl->tofu.batch_update_ref) ctrl->tofu.batch_updated_wanted ++;
ctrl->tofu.batch_update_started = gnupg_get_time ();
ctrl->tofu.batch_update_ref ++;
} }
void void
tofu_end_batch_update (ctrl_t ctrl) tofu_end_batch_update (ctrl_t ctrl)
{ {
log_assert (ctrl->tofu.batch_update_ref > 0); log_assert (ctrl->tofu.batch_updated_wanted > 0);
ctrl->tofu.batch_update_ref --; ctrl->tofu.batch_updated_wanted --;
if (!ctrl->tofu.batch_update_ref) if (!ctrl->tofu.batch_updated_wanted)
end_transaction (ctrl, 1); end_transaction (ctrl, 1);
} }
@ -693,14 +712,13 @@ tofu_closedbs (ctrl_t ctrl)
tofu_dbs_t dbs; tofu_dbs_t dbs;
sqlite3_stmt **statements; sqlite3_stmt **statements;
log_assert(ctrl->tofu.in_transaction == 0); log_assert (ctrl->tofu.in_transaction == 0);
dbs = ctrl->tofu.dbs; dbs = ctrl->tofu.dbs;
if (!dbs) if (!dbs)
return; /* Not initialized. */ return; /* Not initialized. */
if (dbs->batch_update) end_transaction (ctrl, 2);
end_transaction (ctrl, 2);
/* Arghh, that is a surprising use of the struct. */ /* Arghh, that is a surprising use of the struct. */
for (statements = (void *) &dbs->s; for (statements = (void *) &dbs->s;